-
-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Prefer Pathname#glob
to Dir.[]
at spec/rails_helper.rb template
#2678
Conversation
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#glob
was added in Ruby 2.5, I thought it would be more concise to use this on spec/rails_helper.rb template.Unlike
Dir.[]
,Pathname#glob
returns an array ofPathname
, butKernel.#require
works the same way if you give anPathname
as 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.