Use component path for generating RSpec files #2005
Conversation
|
Tests probably aren't a bad idea — I'll get one run up. Good question re breaking change. I suppose it is since it's actively changing generator behaviour... Rails allows overrides of builtin generators, so I can write some documentation about how to set it back to the old way? I can also expand on the changelog entry. Lmk what you think! |
|
@neanias since it's a breaking change, we'll need to either hold it for the next major release or put it behind a config flag. I would vote we do the latter, adding a config flag that defaults to |
|
@joelhawksley Sounds like a plan. Should I put it under the |
|
The config option feels like a bit of a mouthful, but I think it's clear enough? |
|
👋 Nice to see you around here, @neanias! And thanks for a very thorough PR 💯 |
|
Also, worth verifying if we need the same for Minitest. |
|
Just noticed I've used "you" in my documentation for the new config flag. Is that ok or should I change it? |
I'd prefer to avoid "you". Mind making the change? I'll review the language in this PR for ya ❤️ |
|
Ok, I think that's everything? Re the potential equivalent Minitest change, I think that should come under a separate PR |
|
Left "you" in the CHANGELOG 🤦 |
Generally, the path to an RSpec file should match the app path, but substituting `app` for `spec`. If a user changes the generation path for their components, then the generator should use an appropriate path that matches that. In the example where the user changes the generation path from `app/components` to `app/views/components`, the RSpec component spec should be created in `spec/views/components`. The logic chosen here checks that the component is still being generated into the `app` dir, otherwise it will just fallback on the usual destination of `spec/components`. Since this is a breaking change, it is managed by the `use_component_path_for_rspec_tests` config option. NB: Since this includes the `AbstractGenerator`, we can remove the `file_name` method as it's provided by the module.
|
God, really burning through the GHAction minutes with this one! |
|
I can't merge this due to the failing compat check with Primer. Is that expected? |
Co-authored-by: William Mathewson <neanias@users.noreply.github.com>
|
The PVC failure is unrelated. Merging! |
What are you trying to accomplish?
Generally, the path to an RSpec file should match the app path, but substituting
appforspec. If a user changes the generation path for their components, then the generator should use an appropriate path that matches that. In the example where the user changes the generation path fromapp/componentstoapp/views/components, the RSpec component spec should be created inspec/views/components.What approach did you choose and why?
The logic chosen here checks that the component is still being generated into the
appdir, otherwise it will just fallback on the usual destination ofspec/components. I've used simple string matching ofstart_with?("app#{File::SEPARATOR}"), but maybe using something in theFileorPathnameclasses would be more flexible.Anything you want to highlight for special attention from reviewers?
Since this includes the
AbstractGenerator, we can remove thefile_namemethod as it's provided by the module.(Hi, Simon!)