Flaky test: "App: should re-query for executing runs" (windows mainly)

[lmiller1990]

Link to dashboard or CircleCI failure

https://app.circleci.com/pipelines/github/cypress-io/cypress/45562/workflows/8510305c-0f92-41d6-b8fb-2abaa50db11a/jobs/1914521/tests#failed-test-2

Link to failing test in GitHub

cypress/packages/app/cypress/e2e/runs.cy.ts

Lines 888 to 955 in 963e184

	it('should re-query for executing runs', () => {
	cy.get('[data-cy="run-card-icon-RUNNING"]').should('have.length', RUNNING_COUNT).should('be.visible')
	cy.remoteGraphQLIntercept(async (obj) => {
	await new Promise((resolve) => setTimeout(resolve, 100))
	if (obj.result.data?.cloudNode?.newerRuns?.nodes) {
	obj.result.data.cloudNode.newerRuns.nodes = []
	}
	if (obj.result.data?.cloudNodesByIds) {
	obj.result.data?.cloudNodesByIds.map((node) => {
	node.status = 'RUNNING'
	})
	obj.result.data.cloudNodesByIds[0].status = 'PASSED'
	}
	return obj.result
	})
	function completeNext (passed) {
	cy.wrap(obj).invoke('toCall').then(() => {
	cy.get('[data-cy="run-card-icon-PASSED"]').should('have.length', passed).should('be.visible')
	if (passed < RUNNING_COUNT) {
	completeNext(passed + 1)
	}
	})
	}
	completeNext(1)
	})
	it('should fetch newer runs and maintain them when navigating', () => {
	cy.get('[data-cy="run-card-icon-RUNNING"]').should('have.length', RUNNING_COUNT).should('be.visible')
	cy.remoteGraphQLIntercept(async (obj) => {
	await new Promise((resolve) => setTimeout(resolve, 100))
	if (obj.result.data?.cloudNodesByIds) {
	obj.result.data?.cloudNodesByIds.map((node) => {
	node.status = 'PASSED'
	node.totalPassed = 100
	})
	}
	return obj.result
	})
	cy.get('[data-cy="run-card-icon-RUNNING"]').should('have.length', 3).should('be.visible')
	cy.wrap(obj).invoke('toCall')
	cy.get('[data-cy="run-card-icon-PASSED"]').should('have.length', 3).should('be.visible').within(() => {
	cy.get('[data-cy="runResults-passed-count"]').should('contain', 100)
	})
	cy.get('[data-cy="run-card-icon-RUNNING"]').should('have.length', 2).should('be.visible')
	// If we navigate away & back, we should see the same runs
	cy.findByTestId('sidebar-link-settings-page').click()
	cy.remoteGraphQLIntercept((obj) => obj.result)
	moveToRunsPage()
	cy.get('[data-cy="run-card-icon-PASSED"]').should('have.length', 3).should('be.visible')
	cy.get('[data-cy="run-card-icon-RUNNING"]').should('have.length', 2).should('be.visible')
	})
	})

These two are flaky on windows.

Edit: First one is fixed #24833

Analysis

Not sure if it's race condition, I've seen it flake on linux too, but much less.

These tests override window.setTimeout and stub out multiple GraphQL requests - it's quite confusing since there's so much stubbing going on, it's not entirely clear if the flake is in the app code or the test code.

I think we should consider an alternative way to orchestrate these tests that relies less on stubbing, and is more deterministic.

Cypress Version

10.11

Other

It's particularly bad on windows, I can reproduce it locally about 90% of the time.

[lmiller1990]

Works great in linux! :linux master race:

[lmiller1990]

On windows, the stubbed GraphQL response is missing the expected properties. commitInfo, totallDuration etc are null. This is not the case on linux and macOS. I do not know why - I went down to the underlying cross-fetch and node-fetch, everything is fine and present.

At this point, the data is present - this is the last part before the stubbed response is returned.

cypress/packages/frontend-shared/cypress/e2e/e2ePluginSetup.ts

Line 242 in e3de5e7

return new Response(JSON.stringify(result), { status: 200 })

On the other end, in the browser, it's missing properties. I cannot see any other layer in between the above line and the browser where I can debug this, though.

MacOS - has all properties, runs are rendered

Windows - missing properties, not rendered

[lmiller1990]

I tried changing urql to go over HTTP instead of Web Sockets. It works better, but the tests are still not reliable. The way they are written is confusing and unideal. Basically, this page has two ways to get runs:

1. initial query
2. refetch latest w/ a mutation

Here's a screenshot of a bunch of requests running on this page in this test:

The way this is currently handled is conditionals, here (took a long time to figure out exactly how this worked)

cy.remoteGraphQLIntercept(async (obj) => {
        await new Promise((resolve) => setTimeout(resolve, 100))

        if (obj.result.data?.cloudNode?.newerRuns?.nodes) {
          obj.result.data.cloudNode.newerRuns.nodes = []
        }

        if (obj.result.data?.cloudNodesByIds) {
          obj.result.data?.cloudNodesByIds.map((node) => {
            node.status = 'RUNNING'
          })

          obj.result.data.cloudNodesByIds[0].status = 'PASSED'
        }

        return obj.result
      })

It's really difficult to look at these tests and figure out what is going on. You also need to grok we overwrite window.setTimeout, but only for the one polling timeout:

cypress/packages/app/cypress/e2e/runs.cy.ts

Lines 878 to 885 in 05dc4a5

	win.setTimeout = function (fn: () => void, time: number) {
	if (fn.name === 'fetchNewerRuns') {
	obj.toCall = fn
	} else {
	setTimeout(fn, time)
	}
	}
	},

What would be nicer (and what I tried, but no luck, since there's so many requests going, it's hard to reliably intercept and make a deterministic test):

cy.intercept(`query-Runs`, { 
  return [passingRun, runningRun]
})

cy.runQuery()

cy.intercept(`mutation-LatestRuns`, { 
  return [passingRun, passingRun]
})

cy.runMutation()

Something more declarative - I think we need more fine grained control over the GraphQL stuff. Just 'patch the flake' isn't going to cut it - I think we might want to re-examine how we do this testing.

[marktnoonan]

Confirmed this is still passing locally 👍

@lmiller1990 or @warrensplayer do we have an epic for upcoming gql e2e tests?

#23474 seems likely related

[lmiller1990]

Not yet -- upcoming work is more for App<>Cloud testing, not to fix flake in general, but we might get some free flake-fixes for free.

Docs issue: #25653

[jennifer-shehane]

Closing since this is likely stale