-
Notifications
You must be signed in to change notification settings - Fork 982
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
Fixes #35817 - Avoid using fixtures for test settings #9530
Conversation
When running rake snapshots:generate it doesn't load the fixtures. This normally happens in the test setup code, but snapshot generation doesn't use this. This patch uses the settings DSL to set a different default depending on the environment. An additional benefit is that the code is more localized.
Issues: #35817 |
Both of the failures are about the setting table being empty IMHO. |
This mixes test code with production code, which I'm generally against, but the benefits you're describing are outweighing the cons here so 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like failures are related to the changes.
Thank you for your contribution, @ekohl! This PR has been inactive for 3 months, closing for now. |
Reopening the PR, I'm still having the problem on the develop branch. |
I think I've got work blocked waiting for this PR |
Thank you for your contribution! This PR has been inactive for 3 months, closing for now. Feel free to reopen when you return to it. This is an automated process. |
When running rake snapshots:generate it doesn't load the fixtures. This normally happens in the test setup code, but snapshot generation doesn't use this. This patch uses the settings DSL to set a different default depending on the environment. An additional benefit is that the code is more localized.