-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Comprehensive dashboard a11y tests #57821
Conversation
💔 Build FailedTest FailuresKibana Pipeline / kibana-oss-agent / Accessibility Tests.test/accessibility/apps/dashboard·ts.Dashboard add a saved searchStandard Out
Stack Trace
Kibana Pipeline / kibana-xpack-agent / Chrome X-Pack UI Functional Tests.x-pack/test/functional/apps/rollup_job/tsvb·js.rollup app tsvb integration create rollup tsvbStandard Out
Stack Trace
History
To update your PR or re-run it, just comment with: |
test/accessibility/apps/dashboard.ts
Outdated
defaultIndex: 'logstash-*', | ||
}); | ||
await PageObjects.common.navigateToApp('dashboard'); | ||
// await esArchiver.loadIfNeeded('logstash_functional'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this commented out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Earlier round of phase 1 tests used logstash data. I removed it and using sample data. Removed the commented out lines. Thanks @majagrubic
I had a quick look at the failures, and a lot of them are due to elements in visualize/discover/eui. I can fix some of those, but want to make sure someone is not already tackling that, so we don't duplicate the work. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome work! I left a small comment, otherwise good to go.
test/accessibility/apps/dashboard.ts
Outdated
it('Exit out of full screen mode', async () => { | ||
const logoButton = await PageObjects.dashboard.getExitFullScreenLogoButton(); | ||
await logoButton.moveMouseTo(); | ||
await PageObjects.dashboard.clickExitFullScreenTextButton(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have seen the same 3 lines of code in one of the full_screen_mode.js
tests.
How about we wrap it up in a function placing in dashboard_page.ts
:
async function exitFullScreenMode() {
const logoButton = await this.getExitFullScreenLogoButton();
await logoButton.moveMouseTo();
await this.clickExitFullScreenTextButton();
}
If you agree, please update the other test as well
@majagrubic @dmlemeshko thank you. I have addressed the review comments! |
@elasticmachine merge upstream |
@bhavyarm, @majagrubic merged her PR (#58122) which I think superseded this one. Will you take a look to make sure everything you wanted is in master? If so, I think you can close this PR. |
@myasonik thanks. @majagrubic pr is missing one of the test changes I made in a separate test because of Dima's comment(test/functional/apps/dashboard/full_screen_mode.js ). I am going to close this PR - raise a separate one to address it. Thanks! |
meta: #51456
Adding more a11y tests to dashboard app for menu options in edit and view mode