-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Fix flaky calendar test #12760
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
Fix flaky calendar test #12760
Conversation
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.
PR Summary
Fixed flaky calendar tests by making the test more resilient to account access permissions. This change removes snapshot testing in favor of a more flexible response value verification approach.
- Test was failing intermittently due to calendar events being associated with inaccessible accounts
- Modified test to be more permissive while still maintaining validation integrity
- Demonstrates consideration for real-world access control scenarios in testing
- Aligns with best practices for handling permissions in test cases
No files reviewed, no comments
Edit PR Review Bot Settings | Greptile
This reverts commit cab1b5e.
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.
Thanks !
🚀 Preview Environment Ready! Your preview environment is available at: http://bore.pub:20726 This environment will automatically shut down when the PR is closed or after 5 hours. |
@FelixMalfait to update the snapshots to expect the AnyString you would have to run your tests with the flag |
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.
So sometimes it does not match because it returns FIELD_RESTRICTED_ADDITIONAL_PERMISSIONS_REQUIRED
when we don't have permissions instead of the real title, right?
Just realised that we chose to display Subject not shared
in the FE when title is FIELD_RESTRICTED_ADDITIONAL_PERMISSIONS_REQUIRED
, that does not seem to be 100% reliable but that's another topic... 😅
LGTM
@Weiko yes! It's not the prettiest fix but not worth spending time to build something else imo |
Test was flaky because sometimes a calendar event is associated to an account which the user does not have access to
Removing the snapshot to test the exact response value but the test is still there (more flexible)