Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enabling Uptime and Dashboard a11y test #91017

Merged
merged 9 commits into from
Feb 20, 2021
Merged

Conversation

myasonik
Copy link
Contributor

@myasonik myasonik commented Feb 10, 2021

  • Dashboard tests were migrated to using the panel service which performs a lot more "is it safe to click this/wait to click this until it's ready" checks so it should hopefully be more stable now
    • Also skipped a few rules that were being caused by upstream problems that we aren't responsible for
  • Uptime test was reenabled with no changes (recent EUI update should have resolved the issue)
    • Did split one test into two just to make reporting easier to grep

Fixes #90555
Fixes #91592
Fixes #77931
Fixes #77422

@myasonik myasonik changed the title Enabling Uptime a11y test Enabling Uptime and Dashboard a11y test Feb 17, 2021
@myasonik
Copy link
Contributor Author

Flaky test runner results aren't great 😕

In 42 runs:

  • 4 Uptime failures at "alert popover controls"
  • 14 Dashboard failures at "can clone panel "

@myasonik myasonik added release_note:skip Skip the PR/issue when compiling release notes v7.13.0 v8.0.0 labels Feb 18, 2021
@myasonik
Copy link
Contributor Author

Flaky test update:

  • 0 uptime failures 🎉
  • 22 dashboard failures 😢

@myasonik
Copy link
Contributor Author

Flaky test update: 14 dashboard failures 😕

@myasonik
Copy link
Contributor Author

Flaky test update: 7 dashboard failures (and a different error!) 😕

@myasonik
Copy link
Contributor Author

myasonik commented Feb 19, 2021

No flaky tests

🎉 🎉 🎉

@myasonik
Copy link
Contributor Author

@elasticmachine merge upstream

Comment on lines +34 to +46
// EUI bug: https://github.com/elastic/eui/issues/4474
id: 'aria-required-parent',
selector: '[class=*"euiDataGridRowCell"][role="gridcell"] ',
selector: '[class=*"euiDataGridRowCell"][role="gridcell"]',
},
{
// 3rd-party library; button has aria-describedby
id: 'button-name',
selector: '[data-rbd-drag-handle-draggable-id]',
},
{
// EUI bug: https://github.com/elastic/eui/issues/4536
id: 'duplicate-id',
selector: '.euiSuperDatePicker *',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ignore all 3rd party errors because there's nothing we can do directly in Kibana to resolve them until they're resolved upstream.

await testSubjects.click('embeddablePanelMore-mainMenu');
await testSubjects.click('embeddablePanelAction-deletePanel');
it('can delete panel', async () => {
await dashboardPanelActions.removePanel();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use team-maintained page objects if available because sometimes "removing a panel" is more complicated than it seems

@@ -61,7 +60,11 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {

it('overview alert popover controls', async () => {
await uptimeService.overview.openAlertsPopover();
await toasts.dismissAllToasts();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whenever you see an error like #90555 it's probably a toasts issue, even if you're actively opening/closing popovers/flyouts.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

await a11y.testAppSnapshot();
});

it('overview alert popover controls nested content', async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Each snapshot should go into its own test just to make identifying the point of failure easier.

The test name is always easy to find but figuring out where within a test you are can be difficult and axe is basically a black box.

Copy link
Contributor

@dominiqueclarke dominiqueclarke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me from a code perspective. I'm still new but I don't suppose I have to pull down this branch to test since the functional tests, which is what we are changing, are tested against the CI.

@@ -61,7 +60,11 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {

it('overview alert popover controls', async () => {
await uptimeService.overview.openAlertsPopover();
await toasts.dismissAllToasts();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

@myasonik
Copy link
Contributor Author

@elasticmachine merge upstream

@myasonik myasonik added the auto-backport Deprecated: Automatically backport this PR after it's merged label Feb 19, 2021
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@myasonik myasonik merged commit d23c01b into elastic:master Feb 20, 2021
@myasonik myasonik deleted the 90555 branch February 20, 2021 00:21
@kibanamachine
Copy link
Contributor

💔 Backport failed

❌ 7.x: Commit could not be cherrypicked due to conflicts

To backport manually, check out the target branch and run:
node scripts/backport --pr 91017

jloleysens added a commit to jloleysens/kibana that referenced this pull request Feb 22, 2021
…ndition-for-hiding-recommded-allocation

* 'master' of github.com:elastic/kibana: (117 commits)
  [coverage] ingest data in parallel (elastic#92074)
  [Lens] Drag and drop performance improvements (elastic#91641)
  A few more environment uiFilters fixes (elastic#92044)
  Enabling Uptime and Dashboard a11y test (elastic#91017)
  [Security Solution][Detections] Adds more granular validation for nested fields (elastic#92041)
  [Security Solution] [Detections] add overflow-wrap for description (elastic#91945)
  [Security Solution] [Detections] do not truncate filename in value list table in modal (elastic#91952)
  Skip flaky apm test elastic#91673 (elastic#92065)
  [docker] Default server.name to hostname (elastic#90799)
  Use documentation link service for snapshot restore (elastic#91596)
  [Security Solution] Clearing up all jest errors and warnings (elastic#91740)
  Add `@kbn/analytics` to UI Shared Deps (elastic#91810)
  [7.12][Telemetry] Add missing fields for security telemetry (elastic#91920)
  [Security Solution] Adds cypress-pipe (elastic#91550)
  [ML] Fix event rate chart annotation position (elastic#91899)
  [APM] Break down error table api removing the sparklines (elastic#89138)
  docs: update dependencies table bug (elastic#91964)
  [Time to Visualize] Stay in Edit Mode After Dashboard Quicksave (elastic#91729)
  Unskip Search Sessions Management UI test (elastic#90110)
  [Fleet] Handle long text in agent details page (elastic#91776)
  ...

# Conflicts:
#	x-pack/plugins/index_lifecycle_management/__jest__/client_integration/edit_policy/edit_policy.helpers.tsx
#	x-pack/plugins/index_lifecycle_management/__jest__/components/edit_policy.test.tsx
gmmorris added a commit to gmmorris/kibana that referenced this pull request Feb 22, 2021
…bana into task-manager/docs-monitoring

* 'task-manager/docs-monitoring' of github.com:gmmorris/kibana: (29 commits)
  Update docs/developer/plugin-list.asciidoc
  Update docs/api/task-manager/health.asciidoc
  Update docs/api/task-manager/health.asciidoc
  [Lens] Load indexpatterns list from indexPattern Service (elastic#91984)
  [coverage] ingest data in parallel (elastic#92074)
  [Lens] Drag and drop performance improvements (elastic#91641)
  A few more environment uiFilters fixes (elastic#92044)
  Enabling Uptime and Dashboard a11y test (elastic#91017)
  [Security Solution][Detections] Adds more granular validation for nested fields (elastic#92041)
  [Security Solution] [Detections] add overflow-wrap for description (elastic#91945)
  [Security Solution] [Detections] do not truncate filename in value list table in modal (elastic#91952)
  Skip flaky apm test elastic#91673 (elastic#92065)
  [docker] Default server.name to hostname (elastic#90799)
  Use documentation link service for snapshot restore (elastic#91596)
  [Security Solution] Clearing up all jest errors and warnings (elastic#91740)
  Add `@kbn/analytics` to UI Shared Deps (elastic#91810)
  [7.12][Telemetry] Add missing fields for security telemetry (elastic#91920)
  [Security Solution] Adds cypress-pipe (elastic#91550)
  [ML] Fix event rate chart annotation position (elastic#91899)
  [APM] Break down error table api removing the sparklines (elastic#89138)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Feb 22, 2021
* master: (36 commits)
  [Uptime] Thumbnail full screen view steps navigation fix (elastic#91895)
  Implement ScopedHistory.block (elastic#91099)
  [Lens] Fix overlowing content on a chart for charts and table (elastic#92006)
  handle source column differences in embeddable as well (elastic#91987)
  [Vega] [Map] disable map rotation using right right click /  touch rotation gesture (elastic#91996)
  [Lens] Load indexpatterns list from indexPattern Service (elastic#91984)
  [coverage] ingest data in parallel (elastic#92074)
  [Lens] Drag and drop performance improvements (elastic#91641)
  A few more environment uiFilters fixes (elastic#92044)
  Enabling Uptime and Dashboard a11y test (elastic#91017)
  [Security Solution][Detections] Adds more granular validation for nested fields (elastic#92041)
  [Security Solution] [Detections] add overflow-wrap for description (elastic#91945)
  [Security Solution] [Detections] do not truncate filename in value list table in modal (elastic#91952)
  Skip flaky apm test elastic#91673 (elastic#92065)
  [docker] Default server.name to hostname (elastic#90799)
  Use documentation link service for snapshot restore (elastic#91596)
  [Security Solution] Clearing up all jest errors and warnings (elastic#91740)
  Add `@kbn/analytics` to UI Shared Deps (elastic#91810)
  [7.12][Telemetry] Add missing fields for security telemetry (elastic#91920)
  [Security Solution] Adds cypress-pipe (elastic#91550)
  ...
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Feb 23, 2021
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 91017 or prevent reminders by adding the backport:skip label.

@myasonik
Copy link
Contributor Author

Manually backported: #92325

@myasonik myasonik added backported and removed backport missing Added to PRs automatically when the are determined to be missing a backport. labels Feb 23, 2021
myasonik pushed a commit that referenced this pull request Feb 23, 2021
# Conflicts:
#	x-pack/test/accessibility/apps/uptime.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment