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

Enable transactional fixtures in dummy app's specs #943

Merged

Conversation

cgunther
Copy link
Contributor

@cgunther cgunther commented Feb 12, 2025

This wraps each example in a transaction that's rolled back at the end of the example to avoid records leaking from one test to the next.

Without this, if you repeatedly run tests, your database will repeatedly grow, and depending on your assertions, may fail due to data from previous test runs.

CI wouldn't have surfaced this issue as it creates a new DB for each run.

@cgunther cgunther closed this Feb 12, 2025
@cgunther cgunther deleted the test-transactional-fixtures branch February 12, 2025 00:49
@cgunther cgunther restored the test-transactional-fixtures branch February 12, 2025 00:49
@cgunther cgunther reopened this Feb 12, 2025
@cgunther
Copy link
Contributor Author

The Ruby 2.5, 2.6, and 2.7 specs are failing in CI as they all use Rails 6.1, which depends on the concurrent-ruby gem requiring the logger gem, which is no longer the case. This was fixed upstream in Rails (rails/rails#54264), but only backported as far back at Rails 7.0, not 6.1.

If you're intent in keeping support (at least in CI) for Rails 6.1, I can look to conditionally restrict the version of concurrent-ruby used to avoid the issue. Alternatively, you could look to bump the minimum version of Rails supported. Rails itself only supports back to 7.0 for security issues.

@cgunther
Copy link
Contributor Author

cgunther commented Mar 6, 2025

I misspoke on CI not surfacing this issue as I think #944's failures are surfacing it. While CI gets a fresh DB for each run, the records created in each test are likely leaking into the following tests. It's certainly amplified outside CI, as locally your DB would be re-used for each run, not just the tests within a single run, like CI. I suppose it hasn't surfaced in CI as the tests must be coincidentally written in a way where having extra records in the database doesn't accidentally affect the assertions.

This wraps each example in a transaction that's rolled back at the end
of the example to avoid records leaking from one test to the next.

Without this, if you repeatedly run tests, your database will repeatedly
grow, and depending on your assertions, may fail due to data from
previous test runs.

CI wouldn't have surfaced this issue as it creates a new DB for each
run.
@Alexander-Senko Alexander-Senko force-pushed the test-transactional-fixtures branch from 436e432 to 25f5dfe Compare March 6, 2025 14:31
@Alexander-Senko Alexander-Senko merged commit 2f95bbb into drapergem:master Mar 6, 2025
13 checks passed
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.

2 participants