test: final ct-audit tests and component tweaks #19948

marktnoonan · 2022-01-28T13:36:00Z

Closes last outstanding tickets from component testing epic

User facing changelog

This PR addresses some problems noticed when writing tests for some the components, so there are some user-facing changes, mainly for keyboard and screen reader accessibility for our Cards and the Specs List. There is more that can be done, especially to make the main Specs List match the Inline Specs List keyboard behavior, but I think these are still improvements worth having.

Additional details

File by file, this PR:

updates tests checking tabindex of Cards, since we no longer set tabindex on the outer element
updates RowDirectory to render a button so that it can be toggled with keyboard in the Specs List
adds Specs List test to catch Clear Search button bug, check link href, check keyboard interactions
fixes Clear Search bug in Specs List - virtual list was failing after the no-results state because it had no DOM element to render into, v-show preserves the DOM of the list even when hidden by the "no results" state.
prevents "cmd-click" from opening new tabs from specs list links (per Slack convo with Jennifer)
gives directory rows in Specs List a unique ID to manage aria expanded state
makes the rows have an outline more like the Inline Specs List when an item has keyboard focus
adds cypress-real-events to frontend-shared (same version we already have)
adds assertions and states to the Card component test
tweaks card markup for accessibility
adds an assertion to CopyButton. Our usual way of asserting text getting copied doesn't seem to work in component tests, need to look into this
adds component tests for StandardModal. This is tested plenty in E2E but it is nice to gather the expectations in one place and test the props directly.
adds some i18n imports in StandardModal components, and some class re-ordering caused by file save
fixes a random font-weight that didn't get updated to semi bold in the header yet
also fixes some percy snapshot issues caused by having multiple unnamed snapshots in the same test.

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?
[na] Have new configuration options been added to the cypress.schema.json?

cypress-bot · 2022-01-28T13:36:04Z

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.

cypress · 2022-01-28T14:01:49Z

Test summary

17925 • 0 • 211 • 0 • 3

Run details


Project	cypress
Status	Passed
Commit	`4135fe1`
Started	Feb 1, 2022 2:11 PM
Ended	Feb 1, 2022 2:23 PM
Duration	12:10 💡
OS	Linux Debian - 10.10
Browser	Multiple

View run in Cypress Dashboard ➡️

Flakiness

	cypress/e2e/commands/net_stubbing.cy.ts		3
1	network stubbing > intercepting request > should delay the same amount on every response
2	network stubbing > waiting and aliasing > can timeout waiting on a single request using "alias.request"
3	network stubbing > waiting and aliasing > can timeout waiting on a single request using "alias.request"

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

marktnoonan · 2022-01-28T19:42:36Z

packages/frontend-shared/cypress/e2e/support/e2eSupport.ts

@@ -421,6 +421,8 @@ function validateExternalLink (subject, options: ValidateExternalLinkOptions | s
  })
 }

+Cypress.on('uncaught:exception', (err) => !err.message.includes('ResizeObserver loop limit exceeded'))


This is here because in CI there were 16 tests failing due to the error. The error itself is safe to ignore in that it doesn't represent something that blows up the UI and it's debatable that it should be an error instead of a warning. I'm open to suggestions for handling this differently though.

This exception has been a pain in the butt forever.

lmiller1990 · 2022-02-01T01:12:04Z

packages/app/src/specs/SpecsList.vue

+      in order to render the virtual dom. Virtualized lists require stable containers and it
+      can be tricky to debug after this has happened.
+    -->
+    <div


How can a conditional with v-if be stable in the first place (what does stable mean, exactly?)

If we need a DOM node that will never change, we could use the v-once directive.

This is a good point, "stable" was vague. I've moved the comment to be better placed, and updated the wording. The part with v-if is OK coming and going, it's the next element that shouldn't follow the v-if rule. We would have liked to use v-show but it clashed with tailwind's grid class.

Maybe it's overdoing it to have this long comment in the first place, I think some "here be dragons" is useful for future developers, but there are tests now to catch regressions of the specific stuff this fixes.

I still don't fully understand the problem, I guess I need to pull and try it out - but that's okay, the comment should suffice. Hopefully I don't need to edit this anytime soon.

The specific problem that this change fixes is that if the container element is not present in the DOM with when this code runs, it doesn't update the virtualized list, so "clearing the search" from the no-results state was broken, as the old v-if had removed the element needed. @ZachJW34 has some ideas for making this more robust in general when we get a chance. But in this PR I just wanted to fix the bug I found when adding the coverage for that behavior, and give a bit of a here-be-dragons, because some of my first changes seemed to work great locally, but had actually broken the virtualization.

cypress/packages/frontend-shared/src/composables/useVirtualList.ts

Line 84 in 317d759

if (element) {

marktnoonan · 2022-02-01T15:33:01Z

@ZachJW34 @tbiethman I made a couple small changes since yesterday so tagged you to re-review when you get a chance. Tests are green now 🎉

ZachJW34

LGTM!

* 10.0-release: feat: validate specPattern root level (#19980) feat(unify): unsupported browsers (#19997) feat: persist isSideNavigationOpen (#20026) feat: add types for urqlCacheKeys (#20027) fix: rename spec.js to spec.cy.js (#20029) feat: waiting for dependencies to be installed in wizard (#19955) fix: migrate config fields to correct the location (#19940) feat: relaunch browser when switching testing types from app (#19961) test: final ct-audit tests and component tweaks (#19948)

marktnoonan added 12 commits January 26, 2022 20:54

update card markup to be more accessible

0fb7178

give button disabled attr when card is disabled

59ac5ef

switch test from tabindex to checking button

a9d69c5

add some card tests

d6d7248

add cypress-real-events

6a7684e

add cypress-real-events

2805b34

update modal i18n

9cdc062

add standard modal component test

058d92c

font tweak for header version to match rest of header

7b45a10

fix recovery from no result state, improve keyboard behavior

7b33872

specs list tests and a11y tweaks

99fa6b6

avoid chaining off realPress

3c61c47

marktnoonan changed the title ~~update card markup to be more accessible~~ test: final ct-audit tests and component tweaks Jan 28, 2022

marktnoonan added 3 commits January 28, 2022 10:50

Merge branch '10.0-release' into UNIFY-693-last-component-tests

8734f76

merge 10.0-release

f1f9518

add assertion about copy button behavior when clicked

822596f

marktnoonan requested review from tbiethman and ZachJW34 January 28, 2022 16:10

marktnoonan marked this pull request as ready for review January 28, 2022 16:10

Merge branch '10.0-release' into UNIFY-693-last-component-tests

69a9ddd

marktnoonan marked this pull request as draft January 28, 2022 16:25

marktnoonan and others added 6 commits January 28, 2022 11:28

Merge branch '10.0-release' into UNIFY-693-last-component-tests

b0450d6

update project-setup test

4ae677f

Merge branch '10.0-release' into UNIFY-693-last-component-tests

ce0f9c6

update tests

ff36c01

remove copy button test

c19ce91

ignore 'ResizeObserver loop limit exceeded' error

e5eb223

marktnoonan commented Jan 28, 2022

View reviewed changes

marktnoonan added 4 commits January 31, 2022 08:52

fix test typing

50836bf

fix tslint error

cc50696

test removal of full list snapshot

78cc375

remove custom snapshot names

38a99a3

ZachJW34 previously approved these changes Jan 31, 2022

View reviewed changes

test percy name order

e567bd8

marktnoonan dismissed ZachJW34’s stale review via e567bd8 January 31, 2022 16:32

marktnoonan and others added 11 commits January 31, 2022 11:57

percy

8862396

remove specs list snapshots

7d55755

fix vlist issue, restore snapshot names

6765eb8

Merge branch '10.0-release' into UNIFY-693-last-component-tests

422844f

ignore ResizeObserver error in CT

3ecd0dd

ignore ResizeObserver error in CT

2e9359d

minimize calls to calculateRange

317d759

slow down DOM changes in test

bd5ebaa

avoid resetting specs in beforeEach

cb9404b

move exception handler

d8bfbd4

restore old screenshot naming

db89bcc

lmiller1990 reviewed Feb 1, 2022

View reviewed changes

marktnoonan added 4 commits January 31, 2022 20:20

Merge branch '10.0-release' into UNIFY-693-last-component-tests

e583107

fix failing test

556e0e2

update comment

9a5d20f

avoid unnamed snapshots in same test

4135fe1

marktnoonan requested review from tbiethman and ZachJW34 February 1, 2022 14:56

ZachJW34 approved these changes Feb 1, 2022

View reviewed changes

ZachJW34 merged commit 4787cc1 into 10.0-release Feb 1, 2022

ZachJW34 deleted the UNIFY-693-last-component-tests branch February 1, 2022 15:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: final ct-audit tests and component tweaks #19948

test: final ct-audit tests and component tweaks #19948

marktnoonan commented Jan 28, 2022 •

edited

Loading

cypress-bot bot commented Jan 28, 2022

cypress bot commented Jan 28, 2022 •

edited

Loading

marktnoonan Jan 28, 2022

tbiethman Jan 28, 2022

lmiller1990 Feb 1, 2022

marktnoonan Feb 1, 2022

lmiller1990 Feb 1, 2022

marktnoonan Feb 1, 2022

marktnoonan commented Feb 1, 2022

ZachJW34 left a comment

test: final ct-audit tests and component tweaks #19948

test: final ct-audit tests and component tweaks #19948

Conversation

marktnoonan commented Jan 28, 2022 • edited Loading

User facing changelog

Additional details

How has the user experience changed?

PR Tasks

cypress-bot bot commented Jan 28, 2022

cypress bot commented Jan 28, 2022 • edited Loading

Test summary

Run details

Flakiness

marktnoonan Jan 28, 2022

Choose a reason for hiding this comment

tbiethman Jan 28, 2022

Choose a reason for hiding this comment

lmiller1990 Feb 1, 2022

Choose a reason for hiding this comment

marktnoonan Feb 1, 2022

Choose a reason for hiding this comment

lmiller1990 Feb 1, 2022

Choose a reason for hiding this comment

marktnoonan Feb 1, 2022

Choose a reason for hiding this comment

marktnoonan commented Feb 1, 2022

ZachJW34 left a comment

Choose a reason for hiding this comment

marktnoonan commented Jan 28, 2022 •

edited

Loading

cypress bot commented Jan 28, 2022 •

edited

Loading