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

✅ test: Fix failing CSRF tests #2720

Merged
merged 1 commit into from
Nov 22, 2023

Conversation

nickajacks1
Copy link
Member

Description

A test validating that expired tokens fail was hitting a race condition with garbage collection. Sometimes, an assertion that expects memory storage GC to have triggered happens too quickly, causing the assertion to fail. Give the GC a little bit more time to process before asserting.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • I have performed a self-review of my own code
  • New and existing unit tests pass locally with my changes

@nickajacks1
Copy link
Member Author

CI failed on Test_Client_Agent_Dest and Session middleware tests. There were some races reported logs.

A test validating that expired tokens fail was hitting a race condition
with garbage collection. Sometimes, an assertion that expects memory
storage GC to have triggered happens too quickly, causing the assertion
to fail. Give the GC a little bit more time to process before asserting.
@nickajacks1 nickajacks1 force-pushed the testfail-csrf branch 2 times, most recently from 5b39e15 to 697bcde Compare November 19, 2023 21:07
@gaby gaby changed the title ✅ test: fix failing csrf test ✅ test: Fix failing CSRF tests Nov 19, 2023
@gaby gaby removed the 🧹 Updates label Nov 19, 2023
@ReneWerner87 ReneWerner87 added this to the Next v2 Release milestone Nov 22, 2023
@ReneWerner87 ReneWerner87 merged commit eeced20 into gofiber:master Nov 22, 2023
20 checks passed
@nickajacks1 nickajacks1 deleted the testfail-csrf branch November 22, 2023 18:23
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.

4 participants