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

Add more info about a11y tests #76045

Merged
merged 7 commits into from
Sep 3, 2020
Merged

Add more info about a11y tests #76045

merged 7 commits into from
Sep 3, 2020

Conversation

myasonik
Copy link
Contributor

Summary

The accessibility tests look a little different from other functional tests so they raise a few eyebrows. Writing some more docs to better explain them.

@myasonik myasonik added Project:Accessibility v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.10.0 docs labels Aug 26, 2020
bhavyarm
bhavyarm reviewed Aug 26, 2020
View reviewed changes
Copy link
Contributor

@bhavyarm bhavyarm left a comment

Choose a reason for hiding this comment

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

Looks great. Did you also want to add a blurb about skipping tests if the failures cannot be fixed but creating an issue so it gets logged? and then commenting on top of the test with the issue number?

rashmivkulkarni
rashmivkulkarni approved these changes Aug 27, 2020
View reviewed changes
Copy link
Contributor

@rashmivkulkarni rashmivkulkarni left a comment

Choose a reason for hiding this comment

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

LGTM , more docs are always useful. This was missing indeed!

@gchaps
Apply suggestions from code review
c8e076a 
Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com>
@myasonik
Copy link
Contributor Author

Did you also want to add a blurb about skipping tests if the failures cannot be fixed but creating an issue so it gets logged? and then commenting on top of the test with the issue number?

@bhavyarm Do you think that would be helpful? I've seen that type of thing done for any type of test.

@bhavyarm
Copy link
Contributor

@myasonik I keep explaining that process to folks who are adding new tests. Sometimes more than once. So yes? :) I will link this to meta issue on tests too. Thanks!

@myasonik
Copy link
Contributor Author

@bhavyarm @rashminpawar I added the info y'all requested. What do you think?

rashmivkulkarni
rashmivkulkarni approved these changes Aug 27, 2020
View reviewed changes
Copy link
Contributor

@rashmivkulkarni rashmivkulkarni left a comment

Choose a reason for hiding this comment

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

LGTM

cjcenizal
cjcenizal approved these changes Aug 27, 2020
View reviewed changes
Copy link
Contributor

@cjcenizal cjcenizal left a comment

Choose a reason for hiding this comment

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

Very helpful info! Thanks for doing this. Had one question/suggestion.

docs/developer/contributing/development-accessibility-tests.asciidoc Show resolved Hide resolved
@rashmivkulkarni
Copy link
Contributor

btw, @myasonik - pinged the wrong Rashmi :-)

Thanks for adding additional docs with examples

@myasonik myasonik requested a review from gchaps August 28, 2020 00:40
@myasonik
Copy link
Contributor Author

@gchaps Could you look this over one more time? There's been a good amount added in the last few rounds and I think it's ready from a content perspective now.

gchaps
gchaps reviewed Aug 31, 2020
View reviewed changes
docs/developer/contributing/development-accessibility-tests.asciidoc Outdated Show resolved Hide resolved
docs/developer/contributing/development-accessibility-tests.asciidoc Show resolved Hide resolved
docs/developer/contributing/development-accessibility-tests.asciidoc Outdated Show resolved Hide resolved
docs/developer/contributing/development-accessibility-tests.asciidoc Outdated Show resolved Hide resolved
docs/developer/contributing/development-accessibility-tests.asciidoc Outdated Show resolved Hide resolved
docs/developer/contributing/development-accessibility-tests.asciidoc Outdated Show resolved Hide resolved
docs/developer/contributing/development-accessibility-tests.asciidoc Outdated Show resolved Hide resolved
docs/developer/contributing/development-accessibility-tests.asciidoc Outdated Show resolved Hide resolved
docs/developer/contributing/development-accessibility-tests.asciidoc Outdated Show resolved Hide resolved
docs/developer/contributing/development-accessibility-tests.asciidoc Outdated Show resolved Hide resolved
@gchaps
Apply suggestions from code review
f60b996 
Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com>
@myasonik myasonik merged commit 9766969 into elastic:master Sep 3, 2020
@myasonik myasonik deleted the a11y-dev-docs branch September 3, 2020 18:09
jloleysens added a commit to jloleysens/kibana that referenced this pull request Sep 4, 2020
@jloleysens
Merge branch 'master' of github.com:elastic/kibana into ingest-node/g…
8f365fa 
…rok/new-patterns-component-use-array

* 'master' of github.com:elastic/kibana: (75 commits)
  Remove legacy ui-apps-mixin (elastic#76604)
  remove unused test_utils (elastic#76528)
  [ML] Functional tests - add UI permission tests (elastic#76368)
  [APM] @ts-error -> @ts-expect-error (elastic#76492)
  [APM] Avoid negative offset for error marker on timeline (elastic#76638)
  [Reporting] rename interfaces to align with task manager integration (elastic#76716)
  Revert back ESO migration for alerting, added try/catch logic to avoid failing Kibana on start (elastic#76220)
  Test reverting "Add plugin status API (elastic#75819)" (elastic#76707)
  [Security Solution][Detections] Removes ML Job Settings SIEM copy and fixes link to ML app for creating custom jobs (elastic#76595)
  [Maps] remove region/coordinate-maps visualizations from sample data (elastic#76399)
  [DOCS] Dashboard-first docs refresh (elastic#76194)
  Updated ServiceNow description in docs and actions management UI to contains correct info (elastic#76344)
  [DOCS] Identifies cloud settings in reporting (elastic#76691)
  [Security Solution] Refactor timeline details to use search strategy (elastic#75591)
  es-archiver: Drop invalid index settings, support --query flag  (elastic#76522)
  [DOCS] Identifies graph settings available on cloud (elastic#76661)
  Add more info about a11y tests (elastic#76045)
  [data.search.SearchSource] Remove legacy ES client APIs. (elastic#75943)
  [release notes] automatically retry on Github API 5xx errors (elastic#76447)
  [es_ui_shared] Fix eslint exhaustive deps rule (elastic#76392)
  ...
@myasonik myasonik mentioned this pull request Sep 4, 2020
Merged
myasonik pushed a commit to myasonik/kibana that referenced this pull request Sep 4, 2020
@gchaps
Add more info about a11y tests (elastic#76045)
26400dc 
Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com>
myasonik pushed a commit that referenced this pull request Sep 4, 2020
@gchaps
[7.x] Add more info about a11y tests (#76045) (#76805)
60516e7 
Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com>

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

@cjcenizal cjcenizal cjcenizal approved these changes

@gchaps gchaps gchaps left review comments

@rashmivkulkarni rashmivkulkarni rashmivkulkarni approved these changes

@bmcconaghy bmcconaghy Awaiting requested review from bmcconaghy

@bhavyarm bhavyarm Awaiting requested review from bhavyarm

Assignees
No one assigned
Labels
docs Project:Accessibility release_note:skip Skip the PR/issue when compiling release notes v7.10.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@myasonik @bhavyarm @rashmivkulkarni @cjcenizal @gchaps