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

Remove unneeded settings cleanup from specs #28425

Merged

Conversation

ClearlyClaire
Copy link
Contributor

No description provided.

@ClearlyClaire ClearlyClaire added testing Automated lint and test suites ruby Pull requests that update Ruby code labels Dec 19, 2023
Copy link
Contributor

@mjankowski mjankowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the idea here that we don't really care about the global state or default values of any of these within the spec runs -- and where it does matter we can just explicitly set it in a before and then not bother with teardown/cleanup?

@ClearlyClaire
Copy link
Contributor Author

The idea is that we don't need our own teardown/cleanup code, it should be handled by rspec.

@mjankowski
Copy link
Contributor

The idea is that we don't need our own teardown/cleanup code, it should be handled by rspec.

Ah - I see that there's an Rails.cache.clear in an around block at the suite config level ... which is going to reset these all on each example?

@ClearlyClaire
Copy link
Contributor Author

Yeah, those are stored both as a database column and as a Rails cache entry, so everything should be handled by the test transactions and the Rails.cache.clear call already.

@mjankowski
Copy link
Contributor

Cool - I was confused at first because I had these mentally in the same bucket as the Rails.configuration settings -- but yes, given those details this is better and should be safe.

@ClearlyClaire ClearlyClaire requested a review from a team December 19, 2023 15:14
@ClearlyClaire ClearlyClaire added this pull request to the merge queue Dec 19, 2023
Merged via the queue into mastodon:main with commit 6fed0fc Dec 19, 2023
34 checks passed
@ClearlyClaire ClearlyClaire deleted the cleanup/tests-around-blocks branch December 19, 2023 15:22
vmstan pushed a commit to vmstan/mastodon that referenced this pull request Jan 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ruby Pull requests that update Ruby code testing Automated lint and test suites
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants