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.

Sign up for GitHub

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

Jump to bottom

[Upgrade Assistant] Add A11y Tests #90265

Merged
merged 10 commits into from
Feb 9, 2021
Merged

[Upgrade Assistant] Add A11y Tests #90265

merged 10 commits into from
Feb 9, 2021

Conversation

cuff-links
Copy link
Contributor

Closes #80059

This PR adds accessibility functional tests for the Upgrade Assistant app.

It covers:

  • The overview tab
  • The cluster tab
  • The indice tab

@cuff-links cuff-links added Project:Accessibility v8.0.0 Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more release_note:skip Skip the PR/issue when compiling release notes Feature:Upgrade Assistant labels Feb 4, 2021
@cuff-links cuff-links requested a review from a team as a code owner February 4, 2021 09:56
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-accessibility (Project:Accessibility)

@elasticmachine
Copy link
Contributor

Pinging @elastic/es-ui (Team:Elasticsearch UI)

@cuff-links
Copy link
Contributor Author

@elasticmachine merge upstream

alisonelizabeth
alisonelizabeth previously requested changes Feb 4, 2021
View reviewed changes
Copy link
Contributor

@alisonelizabeth alisonelizabeth left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for working on this @cuff-links! It's great to see more accessibility tests.

I left a few questions in the code. I also think you will need to add the test file to x-pack/test/accessibility/config.ts in order for the tests to run in CI.

require.resolve('./apps/upgrade_assistant'),

x-pack/test/accessibility/apps/upgrade_assistant.ts Outdated Show resolved Hide resolved
x-pack/test/accessibility/apps/upgrade_assistant.ts Outdated Show resolved Hide resolved
x-pack/test/accessibility/apps/upgrade_assistant.ts Outdated Show resolved Hide resolved
x-pack/test/accessibility/apps/upgrade_assistant.ts Show resolved Hide resolved
@alisonelizabeth
Copy link
Contributor

I just noticed CI failures and I think there are a couple other things that will need to be addressed:

  • x-pack/test/accessibility/apps/upgrade_assistant.ts will need to be updated with the new license header
  • The jest snapshot for checkup_tab.test.tsx will need to be updated to account for the data-test-subj that was added

@cjcenizal
Copy link
Contributor

@cuff-links This PR needs the 7.12.0 version label if you're planning on backporting to 7.x for the 7.12.0 release.

@cuff-links
Copy link
Contributor Author

@alisonelizabeth This PR wasn't ready yet. I was just pushing some commits. It wasn't supposed to flag you at all. Sorry about that. I am addressing the feedback you left. I was just pushing this more for tracking purposes.

John Dorlus added 2 commits February 4, 2021 19:48
Added data test subjects for all tabs and the detail panel on the ind…
be06851 
…idual pages. Updated tests to wait for detail panel to be visible before taking snapshot.
Merge branch 'Add_A11y_Tests_To_Upgrade_Assistant' of github.com:cuff…
7e05b92 
…-links/kibana into Add_A11y_Tests_To_Upgrade_Assistant
@cuff-links
Copy link
Contributor Author

@elasticmachine merge upstream

@cjcenizal
Copy link
Contributor

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
upgradeAssistant 136.4KB 136.7KB +295.0B

History

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

@cuff-links cuff-links requested a review from myasonik February 5, 2021 19:23
myasonik
myasonik approved these changes Feb 5, 2021
View reviewed changes
Copy link
Contributor

@myasonik myasonik left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

@cuff-links
Copy link
Contributor Author

Thanks for working on this @cuff-links! It's great to see more accessibility tests.

I left a few questions in the code. I also think you will need to add the test file to x-pack/test/accessibility/config.ts in order for the tests to run in CI.

require.resolve('./apps/upgrade_assistant'),

This has been resolved.

alisonelizabeth
alisonelizabeth approved these changes Feb 8, 2021
View reviewed changes
Copy link
Contributor

@alisonelizabeth alisonelizabeth left a comment

Choose a reason for hiding this comment

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

Latest LGTM. Ran the tests successfully locally. Great work @cuff-links!

As a next step, I think it'd be great to add some a11y tests for when there are deprecation issues that need to be addressed (right now, only the empty prompt shows for the indices and cluster tab), but I think this is a great start!

@alisonelizabeth
Copy link
Contributor

@elasticmachine merge upstream

@cuff-links
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
upgradeAssistant 136.4KB 136.7KB +295.0B

History

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

@cuff-links cuff-links mentioned this pull request Feb 8, 2021
Closed
@cuff-links cuff-links merged commit 87212e6 into elastic:master Feb 9, 2021
cuff-links pushed a commit to cuff-links/kibana that referenced this pull request Feb 9, 2021
@kibanamachine
[Upgrade Assistant] Add A11y Tests (elastic#90265)
b5a05a5 
* Added overview a11y test. Added data test subjects to the tabs in the app.

* Added data test subjects for all tabs and the detail panel on the indidual pages. Updated tests to wait for detail panel to be visible before taking snapshot.

* Updated snapshot for upgrade assistant jest tests.

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@cuff-links cuff-links mentioned this pull request Feb 9, 2021
Merged
jloleysens added a commit to jloleysens/kibana that referenced this pull request Feb 9, 2021
@jloleysens
Merge branch 'master' of github.com:elastic/kibana into ilm/simplify-…
a07ad51 
…timeline-and-rollover-info

* 'master' of github.com:elastic/kibana: (47 commits)
  [Fleet] Use TS project references (elastic#87574)
  before/beforeEach clean up (elastic#90663)
  [Vega] user should be able to set a specific tilemap service using the mapStyle property (elastic#88440)
  [Security Solution][Case] ServiceNow SIR Connector (elastic#88655)
  [Search Sessions] Enable extend from management (elastic#90558)
  [ILM] Delete phase redesign (rework) (elastic#90291)
  [APM-UI][E2E] use withGithubStatus step (elastic#90651)
  Add folding in kb-monaco and update some viewers (elastic#90152)
  [Grok Debugger] Changed test to wait for grok debugger container to exist to fix test flakiness (elastic#90543)
  Strongly typed EUI theme for styled-components (elastic#90106)
  Fix vega renovate label (elastic#90591)
  [Uptime] Migrate to TypeScript project references (elastic#90510)
  [Monitoring] Migrate data source for legacy alerts to monitoring data directly (elastic#87377)
  [Upgrade Assistant] Add A11y Tests (elastic#90265)
  [Time to Visualize] Adds functional tests for linking/unlinking panel from embeddable library (elastic#89612)
  [dev-utils/ship-ci-stats] fail when CI stats is down (elastic#90678)
  chore(NA): remove write permissions on Bazel remote cache for PRs (elastic#90652)
  chore(NA): move bazel workspace status from bash script into nodejs executable (elastic#90560)
  Use default ES distribution for functional tests (elastic#88737)
  [Alerts] Jira: Disallow labels with spaces (elastic#90548)
  ...

# Conflicts:
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/timeline/timeline.tsx
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/lib/absolute_timing_to_relative_timing.test.ts
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/lib/absolute_timing_to_relative_timing.ts
cuff-links pushed a commit that referenced this pull request Feb 9, 2021
@kibanamachine
[Upgrade Assistant] Add A11y Tests (#90265) (#90730)
1793d6a 
* Added overview a11y test. Added data test subjects to the tabs in the app.

* Added data test subjects for all tabs and the detail panel on the indidual pages. Updated tests to wait for detail panel to be visible before taking snapshot.

* Updated snapshot for upgrade assistant jest tests.

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alisonelizabeth alisonelizabeth alisonelizabeth approved these changes

@myasonik myasonik myasonik approved these changes

Assignees
No one assigned
Labels
Feature:Upgrade Assistant Project:Accessibility release_note:skip Skip the PR/issue when compiling release notes Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v7.12.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Upgrade assistant needs a11y tests
6 participants
@cuff-links @elasticmachine @alisonelizabeth @cjcenizal @kibanamachine @myasonik