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

Load the model schema when running test in eager load context #49470

Merged
merged 3 commits into from
Oct 4, 2023

Conversation

rafaelfranca
Copy link
Member

Some protections like the one that checks if an enum is pointing to a valid column in the table only works when the database schema is loaded to the model.

Before this change, if schema cache wasn't present and the right combinations of configurations were not set, developers would only see this exception in production.

With this change, those errors would be caught on CI, as soon the tests are loaded.

Some protections like the one that checks if an enum is pointing to
a valid column in the table only works when the database schema is
loaded to the model.

Before this change, if schema cache wasn't present and the right
combinations of configurations were not set, developers would only see
this exception in production.

With this change, those errors would be caught on CI, as soon the
tests are loaded.
@rafaelfranca
Copy link
Member Author

Failures are because our CI sets the CI env, and that has effects in the application as well. I think we should separate those.

Since `load_schema` is private API, we don't want it to be explicitly
called by RSpec.

This feature can be used by RSpec by exposing it in
`rails/testing/maintain_test_schema` so they don't need to reimplement
the same thing.
@rafaelfranca rafaelfranca force-pushed the rm-eager-load-model-schema branch from 4329c95 to aa4e713 Compare October 3, 2023 14:08
@rafaelfranca rafaelfranca force-pushed the rm-eager-load-model-schema branch from 2e3f073 to 90a278e Compare October 3, 2023 14:32
@rafaelfranca rafaelfranca force-pushed the rm-eager-load-model-schema branch 2 times, most recently from ee4aec2 to 18081c6 Compare October 3, 2023 14:48
@rafaelfranca rafaelfranca added this to the 7.1.0 milestone Oct 4, 2023
@rafaelfranca rafaelfranca force-pushed the rm-eager-load-model-schema branch from 18081c6 to 47742fa Compare October 4, 2023 08:32
Right now we are using both to test the Rails applications we generate
and to test Rails itself. Let's keep CI for the app and BUILDKITE to
the framework.
@rafaelfranca rafaelfranca force-pushed the rm-eager-load-model-schema branch from 47742fa to 1f0262a Compare October 4, 2023 09:37
@rafaelfranca rafaelfranca merged commit fed3125 into main Oct 4, 2023
6 checks passed
@rafaelfranca rafaelfranca deleted the rm-eager-load-model-schema branch October 4, 2023 09:51
rafaelfranca added a commit that referenced this pull request Oct 4, 2023
Load the model schema when running test in eager load context
matsales28 added a commit to matsales28/shoulda-matchers that referenced this pull request Nov 10, 2023
This commit fixes the problem of not finding the tables
when running the specs in eager load mode, this was introduced
by Rails 7.1.

Load the model schema when running test in eager load
context rails/rails#49470
matsales28 added a commit to thoughtbot/shoulda-matchers that referenced this pull request Nov 17, 2023
* Add Rails 7.1 to appraisals

* fix: Adjust validate options when managing columns and tables in migration

This commit adjusts the way we manage columns and tables in migrations
to account for the changes in Rails 7.1.0, it was introduced in this
Rails version a validation around the options passed to tables and
columns in migrations, so we need to adjust our code to skip this
validation.

This PR introduced this new validation on the migrations:
rails/rails#46178

* Refine `ActiveRecord::SerializeMatcher` for Rails 7.1 compatibility

This commit refines the `ActiveRecord::SerializeMatcher` to
accommodate changes in the public API introduced in Rails 7.1.
The `serialize` method's API has been updated, necessitating
adjustments to our matcher specs to align with the new API.

While this PR addresses the immediate need for compatibility,
there is potential for further enhancements and refinements in
the matcher's implementation to bring it in line with the revised
public API. However, such improvements will be explored in a future PR.

For reference, the changes in the public API can be found in the following PR: rails/rails#47463

This commit adjusts the `ActiveRecord::SerializeMatcher` the public
API for the `serialize` method has changed on Rails 7.1, so we
had to

* feat: Upadte workflow matrix

* fix: Adjust comparison between hash and `ActionController::Parameters`

The behavior of the comparison between hash and a
`ActionController::Parameters` was changed and now will be deprecated.

Check this Rails PR for more context
rails/rails#44826.

* fix: Adjust usage of `has_secure_token` without a token attr

The behaviour of this method was changed in Rails 7.1, and now
it'll search for a `token` attr in the initialization of the record
instead of the creation. To stay with the same behaviour as before
it's necessary to pass a new argument `on: : create` to the method.

* fix: Adjust specs for primary_key check

* fix: Test updating error_highlight gem

* fix: Add default Rails tables to acceptance spec

This commit fixes the problem of not finding the tables
when running the specs in eager load mode, this was introduced
by Rails 7.1.

Load the model schema when running test in eager load
context rails/rails#49470
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant