test: tweak simulated run mode and some other tests for flake #25623

marktnoonan · 2023-01-30T13:39:30Z

Closes Flaky test: Timed out retrying after 4000ms: Expected not to find content: 'Your tests are loading...' but continuously found it. #23306 and does some further things.

Additional details

#24846 changed the way run mode is determined, which introduced some flake in the cypress-in-cypress-run-mode tests which simulated run mode by modifying window.__CYPRESS_MODE__.

The flake wasn't noticed right away, since the tests usually passed with retries, in a pretty accidental way.

This PR cleans up those tests and a few other instances of flake that were noticed along the way, and changes the way run mode is simulated in the cy-in-cy test. The isRunMode utility function added in #24846 can now return true if a special CY_IN_CY_SIMULATE_RUN_MODE parameter is added to the URL. This removed the race condition on startup between Cypress overriding the global variable and the Vue app initializing that I suspect was at the root of the flake.

This PR does a few other things as well, let's explain them:

allows CloudViewerAndProject and LoginConnectModals to render always in open mode - since the navbar in open mode now shows some cloud data, and avoiding them in the spec-runner was mostly about avoiding graphql in run mode.
modifies a few places in the UI that still checked window.__CYPRESS_MODE__ !== 'run' to use the utils/isRunMode function - this makes things more consistent and makes the cy-in-cy run mode tests behave better
deletes HideDuringScreenshotOrRunMode component - this was hanging around from the time before SpecRunnerOpenMode and SpecRunnerRunMode were split up. It was still used in SpecRunnerOpenMode even though that is never rendered in run mode. This had confusing side effects in the simulated run mode.

Steps to test

Run the tests in packages/app/cypress/e2e/cypress-in-cypress-run-mode.cy.ts locally and make sure they they pass and look correct. Verify we see no flake in CI.

Similarly, check packages/app/src/runner/selector-playground/SelectorPlayground.cy.tsx and see that it passes locally too.

How has the user experience changed?

PR Tasks

Have tests been added/updated?
[na] Has the original issue (or this PR, if no issue exists) been tagged with a release in ZenHub? (user-facing changes only)
[na] Has a PR for user-facing changes been opened in cypress-documentation?
[na] Have API changes been updated in the type definitions?

cypress · 2023-01-30T16:36:58Z

42 flaky tests on run #43602 ↗︎

0	26718	1275	0	42

Details:

Merge branch 'feature/IATR-M0' into marktnoonan/flaky-aut-tests
Project: cypress	Commit: `42342f837e`
Status: Passed	Duration: 18:56 💡
Started: Jan 30, 2023 9:03 PM	Ended: Jan 30, 2023 9:22 PM

commands/net_stubbing.cy.ts • 1 flaky test • 5x-driver-firefox

View Output Video

Test
network stubbing > intercepting request > can delay and throttle a StaticResponse

e2e/origin/commands/waiting.cy.ts • 1 flaky test • 5x-driver-firefox

View Output Video

Test
cy.origin waiting > alias > waits for the route alias to have a response

cypress/cypress.cy.js • 3 flaky tests • 5x-driver-firefox

View Output Video

Test
... > correctly returns currentRetry
... > correctly returns currentRetry
... > correctly returns currentRetry

e2e/origin/cookie_behavior.cy.ts • 4 flaky tests • 5x-driver-electron

View Output Video

Test
... > same site / cross origin > XMLHttpRequest > sets cookie on same-site request if withCredentials is true, and attaches to same-site request if withCredentials is true
... > same site / cross origin > fetch > sets same-site cookies if "include" credentials option is specified from request, but does not attach same-site cookies to request by default (same-origin)
... > same site / cross origin > XMLHttpRequest > sets cookie on same-site request if withCredentials is true, and attaches to same-site request if withCredentials is true
... > same site / cross origin > fetch > sets same-site cookies if "include" credentials option is specified from request, but does not attach same-site cookies to request by default (same-origin)

e2e/origin/navigation.cy.ts • 1 flaky test • 5x-driver-electron

View Output Video

Test
delayed navigation > errors > redirects to an unexpected cross-origin and calls another command in the cy.origin command

The first 5 flaky specs are shown, see all 21 specs in Cypress Cloud.

_{This comment has been generated by cypress-bot as a result of this
project's GitHub integration settings.}

marktnoonan · 2023-01-30T17:00:18Z

packages/app/cypress/e2e/cypress-in-cypress-run-mode.cy.ts

@@ -92,13 +84,3 @@ describe('Cypress In Cypress - run mode', { viewportWidth: 1200 }, () => {
    cy.findByTestId('sidebar').should('not.exist')
  })
 })
-
-function simulateRunModeInUI () {


This always felt hacky, it was OK prior to Dec 2, 2022, but the new approach is a lot more stable than modifying the window since the value will be there before the app initializes at all.

marktnoonan · 2023-01-30T17:01:19Z

packages/app/src/runner/SpecRunnerHeaderRunMode.vue

@@ -32,6 +32,7 @@
          <img
            v-if="selectedBrowser.displayName"
            class="min-w-16px w-16px"
+            alt=""


Might as well treat this image as decorative and hide it from screen readers, the text is right next to it.

The empty alt text will mark it as decorative, but if we want to hide it from readers wouldn't we need something like aria-hidden="true"?

They have the same effect (as would role="presentation"), empty alt is just more standard for various reasons. It predates all the aria stuff.

marktnoonan · 2023-01-30T17:01:53Z

packages/app/src/runner/SpecRunnerOpenMode.vue

@@ -35,7 +35,7 @@
      @panel-width-updated="handlePanelWidthUpdated"
    >
      <template #panel1="{isDragging}">
-        <HideDuringScreenshotOrRunMode


This is changed since we already know for sure it is open mode, the run mode scenario doesn't exist.

warrensplayer · 2023-01-30T17:54:05Z

packages/app/src/runner/selector-playground/SelectorPlayground.cy.tsx

+ * Helper to reset focus for tooltips
+ * jQuery .blur() seemed unreliable, leading to flake
+ */
+function clickAway () {


It would be good to understand more how this fixes the problem better than just the blur does. If this is the best way to handle tooltip dismissals, then perhaps the concept could be encapsulated in a custom cy command.

If the intent is to ensure all tooltips are closed and lose focus there is a special function provided by our chosen tooltip library that allows you to close all open instances - would introduce tighter coupling to the impl but we're already using it in our StandardModal.vue 🤷‍♂️

import { hideAllPoppers } from 'floating-vue' ... hideAllPoppers()

Just floating as a possibility, probably overkill for this scenario

A custom command would make sense in tackling this more broadly.

I can think of a few ways to tease out what is going on, maybe by displaying the mouse position or something, but that's probably a separate task.

mike-plummer · 2023-01-30T18:02:59Z

packages/app/src/runner/SpecRunnerHeaderRunMode.vue

@@ -32,6 +32,7 @@
          <img
            v-if="selectedBrowser.displayName"
            class="min-w-16px w-16px"
+            alt=""


The empty alt text will mark it as decorative, but if we want to hide it from readers wouldn't we need something like aria-hidden="true"?

mike-plummer · 2023-01-30T18:05:17Z

packages/frontend-shared/src/utils/isRunMode.ts

@@ -1 +1 @@
-export const isRunMode = window.__CYPRESS_MODE__ === 'run' && window.top === window
+export const isRunMode = window.location.href.includes('CY_IN_CY_SIMULATE_RUN_MODE') || (window.__CYPRESS_MODE__ === 'run' && window.top === window)


Can we extract CY_IN_CY_SIMULATE_RUN_MODE as a constant? Feels like a we're adding in more magic strings

Constant is a good idea, will do.

mike-plummer · 2023-01-30T18:11:39Z

packages/app/src/runner/selector-playground/SelectorPlayground.cy.tsx

+ * Helper to reset focus for tooltips
+ * jQuery .blur() seemed unreliable, leading to flake
+ */
+function clickAway () {


If the intent is to ensure all tooltips are closed and lose focus there is a special function provided by our chosen tooltip library that allows you to close all open instances - would introduce tighter coupling to the impl but we're already using it in our StandardModal.vue 🤷‍♂️

import { hideAllPoppers } from 'floating-vue' ... hideAllPoppers()

Just floating as a possibility, probably overkill for this scenario

ZachJW34 · 2023-01-30T20:32:57Z

packages/frontend-shared/src/utils/isRunMode.ts

-export const isRunMode = window.__CYPRESS_MODE__ === 'run' && window.top === window
+import { CY_IN_CY_SIMULATE_RUN_MODE } from '@packages/types/src/constants'
+
+export const isRunMode = window.location.href.includes(CY_IN_CY_SIMULATE_RUN_MODE) || (window.__CYPRESS_MODE__ === 'run' && window.top === window)


Could this be another window variable? We could set something like window.__CYPRESS_CY_IN_CY_SIMULATE_RUN_MODE. I think the implementation of this would be a bit more involved since you might have to funnel this variable into the serving of the app index.html but the benefit would be that it would fit into the existing strategy. With this we now have state stored on the window and the URL

Initially it was on the window and like you picked up on, Cypress setting it is too late in the process, so I think you're right, we'd have to modify the page itself before it's served, or hook in earlier so that the page renders in run mode.

I think it's OK to use the URL to tell a page to start out in a particular state. It's also kinda neat if you are in actual open mode and want to flip to "simulated run mode".

Opening to revisiting at any time.

The toggling between seems cool! Good way to test the run mode vs open mode UI.

Mayben window:before:load could have worked, will test that out. But yeah the toggling might actually be easier than running --headed and having to restart Cypress in some situations

warrensplayer and others added 7 commits January 27, 2023 14:57

fix: debug dismiss filter and sidebar on runner

d58b95f

update runmode simulation in test

3ae9878

add empty alt for decorative image

ec7a16e

improve stability of test

59ad617

simulate run mode via url, cleanup

e056cbd

run ci

b7050e8

Merge branch 'feature/IATR-M0' into marktnoonan/flaky-aut-tests

88c3102

marktnoonan marked this pull request as ready for review January 30, 2023 16:54

marktnoonan requested review from ZachJW34 and a team January 30, 2023 16:54

marktnoonan changed the title ~~fix: tweak simulated run mode and some other tests for flake~~ test: tweak simulated run mode and some other tests for flake Jan 30, 2023

marktnoonan commented Jan 30, 2023

View reviewed changes

warrensplayer approved these changes Jan 30, 2023

View reviewed changes

warrensplayer requested a review from a team January 30, 2023 17:54

mike-plummer reviewed Jan 30, 2023

View reviewed changes

move CY_IN_CY_SIMULATE_RUN_MODE into constant

888236a

mike-plummer approved these changes Jan 30, 2023

View reviewed changes

ZachJW34 reviewed Jan 30, 2023

View reviewed changes

Merge branch 'feature/IATR-M0' into marktnoonan/flaky-aut-tests

42342f8

ZachJW34 approved these changes Jan 30, 2023

View reviewed changes

warrensplayer merged commit dce4759 into feature/IATR-M0 Jan 30, 2023

warrensplayer deleted the marktnoonan/flaky-aut-tests branch January 30, 2023 21:28

cypress bot mentioned this pull request Jan 30, 2023

feat: Debug page [IATR] #25488

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: tweak simulated run mode and some other tests for flake #25623

test: tweak simulated run mode and some other tests for flake #25623

marktnoonan commented Jan 30, 2023 •

edited

Loading

cypress bot commented Jan 30, 2023 •

edited

Loading

marktnoonan Jan 30, 2023

marktnoonan Jan 30, 2023

mike-plummer Jan 30, 2023

marktnoonan Jan 30, 2023

marktnoonan Jan 30, 2023

warrensplayer Jan 30, 2023

mike-plummer Jan 30, 2023

marktnoonan Jan 30, 2023

mike-plummer Jan 30, 2023

mike-plummer Jan 30, 2023

marktnoonan Jan 30, 2023

mike-plummer Jan 30, 2023

ZachJW34 Jan 30, 2023

marktnoonan Jan 30, 2023

ZachJW34 Jan 30, 2023

marktnoonan Jan 30, 2023

		@@ -1 +1 @@
		export const isRunMode = window.__CYPRESS_MODE__ === 'run' && window.top === window
		export const isRunMode = window.location.href.includes('CY_IN_CY_SIMULATE_RUN_MODE') \|\| (window.__CYPRESS_MODE__ === 'run' && window.top === window)

test: tweak simulated run mode and some other tests for flake #25623

test: tweak simulated run mode and some other tests for flake #25623

Conversation

marktnoonan commented Jan 30, 2023 • edited Loading

Additional details

Steps to test

How has the user experience changed?

PR Tasks

cypress bot commented Jan 30, 2023 • edited Loading

42 flaky tests on run #43602 ↗︎

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marktnoonan commented Jan 30, 2023 •

edited

Loading

cypress bot commented Jan 30, 2023 •

edited

Loading