test: Addressing launchpad test flake in Windows #22536

tbiethman · 2022-06-27T17:21:46Z

This PR addresses the consistently failing spec for the launchpad's global mode tests in Windows CI. See comments for details.

Steps to test

Open up the launchpad, navigate to a project, and select a testing type
Navigate back to the testing type selection
Select a testing type again (the same one or different, doesn't matter)
Inspect the project name link in the nav breadcrumbs and make sure there is no role attribute set on it (we want the implicit role on the a tag to take priority in this case)

How has the user experience changed?

PR Tasks

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

…onfigured

cypress-bot · 2022-06-27T17:21:50Z

Thanks for taking the time to open a PR!

Create a Draft Pull Request if your PR is not ready for review. Mark the PR as Ready for Review when you're ready for a Cypress team member to review the PR.
Become familiar with the Code Review Checklist for guidelines on coding standards and what needs to be done before a PR can be merged.

tbiethman · 2022-06-27T17:23:39Z

system-tests/projects/todos/cypress/support/component-index.html

@@ -0,0 +1,12 @@
+<!DOCTYPE html>


I noticed a bunch of compilation errors around the failing spec due to this file missing from the todos project. todos is configured for component testing but did not include this file. I don't think this was causing a problem in the tests outside of the chatty log, but now it's set up correctly and the log is chatty no more.

tbiethman · 2022-06-27T17:25:11Z

packages/launchpad/cypress/e2e/global-mode.cy.ts

@@ -152,7 +152,8 @@ describe('Launchpad: Global Mode', () => {

    it('updates breadcrumb when selecting a project and navigating back', () => {
      const getBreadcrumbLink = (name: string, options: { disabled: boolean } = { disabled: false }) => {
-        return cy.findByRole('link', { name }).should('have.attr', 'aria-disabled', options.disabled ? 'true' : 'false')
+        // The timeout is increased to account for variability in configuration load times in CI.
+        return cy.findByRole('link', { name, timeout: 10000 }).should('have.attr', 'aria-disabled', options.disabled ? 'true' : 'false')


In addition to the role lookup failing, the test would occasionally have to retry due to the config loading taking a bit longer than expected. So I just bumped the timeout here to give us plenty of room.

cypress · 2022-06-27T17:32:13Z

Test summary

4377 • 0 • 49 • 0 • 0

Run details


Project	cypress
Status	Passed
Commit	`c3b0858`
Started	Jun 27, 2022 6:58 PM
Ended	Jun 27, 2022 7:10 PM
Duration	12:07 💡
OS	Linux Debian - 10.11
Browser	Electron 100

View run in Cypress Dashboard ➡️

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

tbiethman · 2022-06-27T18:38:14Z

packages/frontend-shared/src/gql-components/HeaderBarContent.vue

              <a
+                :key="Boolean(hasLinkToCurrentProject).toString()"


I noticed that the failure here was that findByRole couldn't find the breadcrumb links appropriately: https://dashboard.cypress.io/projects/sehy69/runs/12235/overview/83c7515d-deb1-414a-953c-b74c08865063

I could see in the recording that the links are enabled. And findByRole did find the 'Projects' and 'todos' elements, but it reported their role as...nothing?

listitem: Name "": Name "": Name "": Name "": Name "": --- : // <-- Wat Name "Projects": Name "todos": --- link: Name "v10.2.0":

I found this odd, so I took a look at those elements as rendered in the test and noticed that as we transitioned through the lifecycle, we would end up in a state where href/role attrs have gone from unset, to set, to unset again. However, once set, they would not be removed from the rendered element. So we would end up in a state where we have an a tag, with an href attribute, and a blank role set on it explicitly.

So long story short, I added a key to make sure these attributes we don't want to set actually do get removed. I tried using null rather than undefined in these cases to try and get these attributes to be completely removed from the same element instance, with no success.

And what do you know, this change fixed the test. As it turns out, our windows and linux builds are running these tests on different versions of Chrome. Windows is using Chrome 103 (the most recent build that it downloads onto machine at test time) vs. the linux build's Chrome 100 it uses from the docker image. This spec is actually failing in develop with Chrome 103, regardless of platform.

So even longer story short, correcting the issues with the blank role fixes this test and leaves us with a more correct DOM.

Excellent! And ugh. What a strange combination of events. I'm also real surprised that explicitly setting undefined which is also the natural value for the role makes a difference, as it didn't seem to make any difference to Chrome's accessibility tree. But hurray for getting these passing.

As a side note, I'm suggesting we just not have this link at all any more. If anybody has thoughts one way or the other.

It looks like previous versions of chrome would leave the "link" value for the attribute, even when we unset it. This would allow the test to succeed, but it's not what we were wanting/expecting DOM-wise.

Chrome 103 gets a little closer and removes the role value from the element, but leaves the attribute just hanging there, like a boolean attribute: <div role>Wat</div>. This....is worse functionally but more indicative of what we're wanting, which is to unset the role.

I too would have figured that setting undefined would have cleared it, but I imagine this is some virtual dom weirdness where it's hard to interpret "no value" and "no attribute at all" through a single API.

rachelruderman · 2022-06-27T18:47:51Z

What a rockstar, thank you!! Do you mind filling out Steps to test? Given the comments I imagine in this case it's as simple as "Run packages/launchpad/cypress/e2e/global-mode.cy.ts on Chrome 103 and confirm it passes`, but would be great to have in the PR description for posterity

tbiethman · 2022-06-27T18:56:41Z

@rachelruderman Steps to test have been added!

tgriesser · 2022-06-27T19:15:33Z

Amazing

tgriesser

🎉

@marktnoonan

…esser/CLOUD-577-spec-list-display-latest-runs-batching * muaz/CLOUD-577-spec-list-display-latest-runs: test: Addressing launchpad test flake in Windows (#22536) address comments from @marktnoonan Address code review comments followup on other comments re: @lmiller1990 PR comments chore(deps): update dependency semantic-release to v19 [security] (#22238) chore: Address skipped specs in server package (#22356) Address code review findings Address code review findings Empty-Commit to generate new percy nonce fix: handle case of implicit plugins/index.js files during migration (#22501)

tbiethman added 4 commits June 27, 2022 09:25

test: Bumping timeout on breadcrumb state checks for Windows

3a7f2a3

todos project was missing component index but has component testing c…

36df684

…onfigured

Adding key to links to ensure attrs are cleaned up/reset

43a3390

Coercing boolean keys to strings to match vue key types

c3b0858

tbiethman commented Jun 27, 2022

View reviewed changes

tbiethman marked this pull request as ready for review June 27, 2022 18:40

tbiethman requested review from a team as code owners June 27, 2022 18:40

tbiethman requested review from emilyrohrbough and removed request for a team June 27, 2022 18:40

BlueWinds approved these changes Jun 27, 2022

View reviewed changes

rachelruderman approved these changes Jun 27, 2022

View reviewed changes

ZachJW34 approved these changes Jun 27, 2022

View reviewed changes

tgriesser self-requested a review June 27, 2022 19:16

tgriesser approved these changes Jun 27, 2022

View reviewed changes

tbiethman merged commit 24052eb into develop Jun 27, 2022

tbiethman deleted the tbiethman/windows-flake-6-27 branch June 27, 2022 19:24

lmiller1990 mentioned this pull request Dec 19, 2022

Cypress is running really slow after upgrading to v10 #22868

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: Addressing launchpad test flake in Windows #22536

test: Addressing launchpad test flake in Windows #22536

tbiethman commented Jun 27, 2022 •

edited

cypress-bot bot commented Jun 27, 2022

tbiethman Jun 27, 2022

tbiethman Jun 27, 2022

cypress bot commented Jun 27, 2022 •

edited

tbiethman Jun 27, 2022 •

edited

marktnoonan Jun 27, 2022

tbiethman Jun 27, 2022

rachelruderman commented Jun 27, 2022

tbiethman commented Jun 27, 2022

tgriesser commented Jun 27, 2022

tgriesser left a comment

test: Addressing launchpad test flake in Windows #22536

test: Addressing launchpad test flake in Windows #22536

Conversation

tbiethman commented Jun 27, 2022 • edited

Steps to test

How has the user experience changed?

PR Tasks

cypress-bot bot commented Jun 27, 2022

tbiethman Jun 27, 2022

Choose a reason for hiding this comment

tbiethman Jun 27, 2022

Choose a reason for hiding this comment

cypress bot commented Jun 27, 2022 • edited

Test summary

Run details

tbiethman Jun 27, 2022 • edited

Choose a reason for hiding this comment

marktnoonan Jun 27, 2022

Choose a reason for hiding this comment

tbiethman Jun 27, 2022

Choose a reason for hiding this comment

rachelruderman commented Jun 27, 2022

tbiethman commented Jun 27, 2022

tgriesser commented Jun 27, 2022

tgriesser left a comment

Choose a reason for hiding this comment

tbiethman commented Jun 27, 2022 •

edited

cypress bot commented Jun 27, 2022 •

edited

tbiethman Jun 27, 2022 •

edited