Prefer Pathname#glob to Dir.[] at spec/rails_helper.rb template#2678
Prefer Pathname#glob to Dir.[] at spec/rails_helper.rb template#2678JonRowe merged 1 commit intorspec:mainfrom
Pathname#glob to Dir.[] at spec/rails_helper.rb template#2678Conversation
|
Thanks for the suggestion, merged because I can't see why not. |
|
Released in 6.1.0 |
It turns out this affects sorting.
Not sure if we just do weird things in our support scripts ( |
|
Wondering what RuboCop guys would say about this. I’d expect the order to remain the same as it was with Dir.[], and dir.rb to come first. The latter seems to do with the Pathname itself. Wondering if this is the first time the combination of that cop with order-dependency bites someone? |
|
@pirj The ordering as returned by |
|
We could change |
That’s what I meant. But indeed the cop is not directly responsible due to us calling sort on the results. |
|
@JonRowe I'm happy to send a PR. @pirj I think the fact that Random idea: We could check out how Zeitwerk determines the order when eager loading a directory. |
|
https://github.com/ruby/pathname/blob/9d0190017f1ade0850bb6c47d8cf0c4165ea1dc0/test/pathname/test_pathname.rb#L540 is not too verbose. Can we call the behaviour in our case undetermined?
What if it just returned the same reverse ordered list like z, y, x? |
Before rspec#2678, the suggested code snippet for loading support files loaded files in directories (e.g. `spec/support/a/b.rb`) *after* files with the same basename as those directories (e.g. `spec/support/a.rb`). This tends to be the preferrable load order, given how Ruby modules and classes are usually structured. The root cause is a difference between the alphabetic ordering of string sorting compared to `Pathname#<=>`.
Since
Pathname#globwas added in Ruby 2.5, I thought it would be more concise to use this on spec/rails_helper.rb template.Unlike
Dir.[],Pathname#globreturns an array ofPathname, butKernel.#requireworks the same way if you give anPathnameas an argument, so I think there should be no problem about this difference here. We could add.map(&:to_s), but I think that would be a bit redundant.