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

Clear cache users between tests #342

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

geekingfrog
Copy link
Contributor

This must be done for data cases as well as server cases. Because most of the querying logic rely under the hood on the caches, when someone creates a user, it won't be persisted to the DB after the test, but the cache will not be rolled back.

This must be done for data cases as well as server cases.
Because most of the querying logic rely under the hood on the caches,
when someone creates a user, it won't be persisted to the DB after the
test, but the cache will not be rolled back.
@geekingfrog geekingfrog marked this pull request as ready for review June 26, 2024 15:34
@jauggy
Copy link
Member

jauggy commented Jun 28, 2024

I tested and it worked with this test:
https://github.com/beyond-all-reason/teiserver/blob/f6421ac748e1a215d61959cc9fafea6144717288/test/teiserver/common/clear_db_each_test.exs

Would be better if some sort of test was included in this PR.

@geekingfrog
Copy link
Contributor Author

I've added a test in f543bcc. Not sure of its value but it's something at least.

@jauggy
Copy link
Member

jauggy commented Jul 1, 2024

Yeah I was thinking of the add user test that caused me to find the bug in the first place.

@geekingfrog geekingfrog force-pushed the geekingfrog/clear-user-cache-between-tests branch from f543bcc to dedf5a4 Compare July 1, 2024 14:23
@geekingfrog
Copy link
Contributor Author

Alright, I amended the commit and ended up using your test file. Not the biggest fan, but it's a start and definitely better than nothing.

@geekingfrog geekingfrog mentioned this pull request Jul 5, 2024
6 tasks
@geekingfrog
Copy link
Contributor Author

I'm going to hold off merging that, there's something fishy going on with some tests :(
async + global caches => weird shit happening

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants