fix various Cypress race conditions in Privacy Center #5040
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
Passing run #8636 ↗︎
Details:
Review all test suite changes for PR #5040 ↗︎ |
|||||||||||||||
NevilleS
approved these changes
Jul 1, 2024
Comment on lines
+41
to
+44
| env: { | ||
| // race condition sometimes picks up env variable *after* the override, so let's be double sure | ||
| FIDES_PRIVACY_CENTER__IS_OVERLAY_ENABLED: true, | ||
| }, |
There was a problem hiding this comment.
What does this do? I can't really find any references for the it() function...
There was a problem hiding this comment.
if it has 3 parameters, the 2nd one is for options. env is one of the options allowed for setting an environment variable for this test only.
see https://docs.cypress.io/guides/guides/environment-variables#Option-5-Test-Configuration
Comment on lines
403
to
404
| cy.window(); | ||
| cy.window().then((win) => { |
There was a problem hiding this comment.
Is the double .window() intentional to make this less flakey?
There was a problem hiding this comment.
I think this might be a copy/pasta issue. I'll double check 😅
There was a problem hiding this comment.
removed! Good catch.
Comment on lines
+1635
to
+1640
| // Consent reporting intercept | ||
| cy.intercept( | ||
| "PATCH", | ||
| `${API_URL}/consent-request/consent-request-id/notices-served`, | ||
| { fixture: "consent/notices_served.json" } | ||
| ).as("patchNoticesServed"); |
gilluminate
force-pushed
the
gill/cypress-race-conditions
branch
from
7afa1ce
to
801123b
Compare
Passing run #8637 ↗︎
Details:
Review all test suite changes for PR #5040 ↗︎ |
|||||||||||||||
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description Of Changes
Running Cypress locally has caused some level of developer frustration. Some tests require messing with environment variables to get them to behave like the CI tests, others fail randomly due to race conditions. These fixes help make everything more consistent, and thereby help give more confidence in the tests as we work.
Code Changes
patchNoticesServedintercept forconsent-i18n.cy.tsin to thebeforeEachsection to make sure it's ready prior to the test running at all. This was one that was randomly failing.isOverlayEnabledto be false on a particular consent test that was previously requiring developers to set tofalsemanually before running. No longer need to update.envto run tests!isOverlayEnabledto be true on a test that would then fail when trying to update.envto make the previous test work. This one would also randomly fail without any.envadjustments due to the runner picking up env variable after the override sometimes.Steps to Confirm
Pre-Merge Checklist
CHANGELOG.md