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.

Sign up for GitHub

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

Jump to bottom

Use @DirtiesContext with our own integration tests #38236

Closed
philwebb opened this issue Nov 6, 2023 · 5 comments
Closed

Use @DirtiesContext with our own integration tests #38236

philwebb opened this issue Nov 6, 2023 · 5 comments
Assignees
@mhalbritter
Labels
status: declined A suggestion or change that we don't feel we should currently apply

Comments

@philwebb
Copy link
Member

philwebb commented Nov 6, 2023

Our own integration tests are not likely to benefit from context caching so we could apply @DirtiesContext more liberally. This would also help prevent issues such as #38176
@philwebb philwebb added the type: task A general task label Nov 6, 2023
@philwebb philwebb added this to the 3.2.x milestone Nov 6, 2023
@mhalbritter
Copy link
Contributor

mhalbritter commented Nov 7, 2023 •
edited
Loading

We could also set spring.test.context.cache.maxSize=0 to effectively disable the cache. See here.

@wilkinsona wilkinsona changed the title Use @DirtiesContext with our own intergration tests Use @DirtiesContext with our own integration tests Nov 7, 2023
@philwebb
Copy link
Member Author

philwebb commented Nov 7, 2023

That might be a good option for most modules. I seem to remember we have some tests that might test context caching but I'm not sure a which level they work.

@mhalbritter
Copy link
Contributor

Ah, spring.test.context.cache.maxSize can't be set to 0, it must be positive.

mhalbritter added a commit that referenced this issue Nov 8, 2023
@mhalbritter
Remove system property dependency from MockBeanContextCachingTests
66ed4fe 
Now this test can be run regardless of the
'spring.test.context.cache.maxSize' system property value.

See gh-38236
@mhalbritter mhalbritter self-assigned this Nov 9, 2023
@mhalbritter mhalbritter added the for: team-meeting An issue we'd like to discuss as a team to make progress label Nov 9, 2023
@mhalbritter
Copy link
Contributor

I've labelled it as team meeting as we should talk about if that brings any benefit. Context caching is on by default for all users, if we disable it, we might not catch some bugs that would occur if it's switched on.

@philwebb philwebb closed this as not planned Won't fix, can't repro, duplicate, stale Nov 9, 2023
@philwebb philwebb added status: declined A suggestion or change that we don't feel we should currently apply and removed type: task A general task for: team-meeting An issue we'd like to discuss as a team to make progress labels Nov 9, 2023
@philwebb philwebb removed this from the 3.2.x milestone Nov 9, 2023
@philwebb
Copy link
Member Author

philwebb commented Nov 9, 2023

After some discussion we decided not to do this. We want our tests to reflect a typical user setup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mhalbritter mhalbritter

Labels
status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@philwebb @mhalbritter