Skip to content
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

CI against Rails 7.1 #429

Closed
wants to merge 3 commits into from
Closed

CI against Rails 7.1 #429

wants to merge 3 commits into from

Conversation

y-yagi
Copy link
Contributor

@y-yagi y-yagi commented Oct 27, 2023

This PR adds Rails 7.1 to the CI matrix to ensure the gem works with Rails 7.1.

This PR includes two commits to pass the test.

  • Ignore the directory that keeps factory files under the lib
    Some tests put factory files under the lib. But since Rails 7.1, Rails loads lib directory by default in a new application.
    https://guides.rubyonrails.org/7_1_release_notes.html#introducing-config-autoload-lib-and-config-autoload-lib-once-for-enhanced-autoloading
    But factory files don't follow the naming rule of Zeitwerk. So Zeitwerk raises Zeitwerk::NameError. To avoid the error, this
    changed to ignore the directory that puts factory files.

  • Remove the route that is defined by default
    Some test code loads the route file twice. This wasn't a problem before Rails 7.0 because we didn't have a route.
    But, Rails 7.1 has one route by default. So the test application raises the following error during the load.

    Invalid route name, already in use: 'rails_health_check'  (ArgumentError)
    You may have defined two routes with the same name using the `:as` option, or you may be overriding a route already 
    defined by a resource with the same naming. For the latter, you can restrict the routes created with `resources` as explained 
    here:
    https://guides.rubyonrails.org/routing.html#restricting-the-routes-created
    

    The routing isn't a matter of this gem, so just ignore that for running the test.

@y-yagi y-yagi force-pushed the rails-71 branch 4 times, most recently from 39673ba to c5228bb Compare October 27, 2023 04:50
Some tests put factory files under the `lib`. But since Rails 7.1,
Rails loads `lib` directory by default in a new application.
https://guides.rubyonrails.org/7_1_release_notes.html#introducing-config-autoload-lib-and-config-autoload-lib-once-for-enhanced-autoloading

But factory files don't follow the naming rule of Zeitwerk. So
Zeitwerk raises `Zeitwerk::NameError`.

To avoid the error, this changed to ignore the directory that puts
factory files.
Some test code loads the route file twice. This wasn't a problem
before Rails 7.0 because we didn't have a route.

But, Rails 7.1 has one route by default. So the test application
raises the following error during the load.

```
Invalid route name, already in use: 'rails_health_check'  (ArgumentError)
You may have defined two routes with the same name using the `:as` option, or you may be overriding a route already defined by a resource with the same naming. For the latter, you can restrict the routes created with `resources` as explained here:
https://guides.rubyonrails.org/routing.html#restricting-the-routes-created
```

The routing isn't a matter of this gem, so just ignore that for
running the test.
@y-yagi y-yagi marked this pull request as ready for review October 27, 2023 05:18
@neilvcarvalho
Copy link
Member

This PR's commits were cherry-picked to #440, and released on https://github.com/thoughtbot/factory_bot_rails/releases/tag/v6.4.2

Thank you for working on this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants