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

Unskip ML a11y tests #154001

Closed
wants to merge 2 commits into from
Closed

Unskip ML a11y tests #154001

wants to merge 2 commits into from

Conversation

bhavyarm
Copy link
Contributor

@bhavyarm bhavyarm commented Mar 29, 2023

Awesome ML team fixed their a11y bugs - #153596 & #153592 which I logged while doing axe-core engine update

Unskipping ML a11Y tests

@bhavyarm bhavyarm self-assigned this Mar 29, 2023
@bhavyarm bhavyarm added Project:Accessibility v8.8.0 release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting :ml labels Mar 29, 2023
@pheyos
Copy link
Member

pheyos commented Mar 30, 2023

It looks like with the provided fix, we now run into some follow-up failures. I've tracked that down to the following items (everything else failing looks just to be a result from earlier failures and thus incomplete test steps - the run doesn't stop on the first failure).

1. Link issue on the overview page

1)    ml Accessibility
       for user with full ML access
         with no data loaded
           overview page:

      Error: a11y report:
 
 VIOLATION
   [link-in-text-block]: Ensure links are distinguished from surrounding text in a way that does not rely on color
     Impact: serious
     Help: https://dequeuniversity.com/rules/axe/4.6/link-in-text-block?application=axeAPI
     Elements:
       - <a class="euiLink css-11bjcda-euiLink-accent" href="/app/management/data/transform" target="blank" rel="noreferrer">transforms</a>
       - <a class="euiLink css-11bjcda-euiLink-accent" href="https://www.elastic.co/guide/en/machine-learning/master/ml-dfa-overview.html#prepare-transform-data" target="blank" rel="noreferrer">

2. The same link issue, coming up on the validation step in all AD job wizards

1)    ml Accessibility
       for user with full ML access
         with data loaded
           anomaly detection create multi metric job validation step:

      Error: a11y report:
                                                                                                                  
 VIOLATION                                                                                                        
   [link-in-text-block]: Ensure links are distinguished from surrounding text in a way that does not rely on color
     Impact: serious
     Help: https://dequeuniversity.com/rules/axe/4.6/link-in-text-block?application=axeAPI
     Elements:
       - <a class="euiLink css-zxmf4e-euiLink-primary" href="https://www.elastic.co/guide/en/machine-learning/master/ml-ad-run-jobs.html#ml-ad-influencers" target="_BLANK" rel="noreferrer">Learn more</a>

3. Issue with a select name on the single metric viewer page

1)    ml Accessibility
       for user with full ML access
         with data loaded
           anomaly detection forecasting from Single Metric Viewer page:

      Error: a11y report:
 
 VIOLATION
   [select-name]: Ensures select element has an accessible name
     Impact: critical
     Help: https://dequeuniversity.com/rules/axe/4.6/select-name?application=axeAPI
     Elements:
       - <select class="euiSelect euiFormControlLayout--1icons euiSelect--compressed euiSelect--inGroup" data-test-subj="mlAnomalyIntervalControls"><option value="auto">Auto</option><option value="hour">1 hour</option><option value="day">1 day</option><option value="second">Show all</option></select>

@pheyos
Copy link
Member

pheyos commented Mar 30, 2023

AFAIU @bhavyarm will check items 1 and 2 as this was coming up before.
Item 3 looks like a legit one at glance and might require an adjustment in the ML code.
FYI @peteharverson

@bhavyarm
Copy link
Contributor Author

cc @1Copenut

Link issue on the overview page

Issue 1 and 2 - FYI. Thanks!

@peteharverson
Copy link
Contributor

Thanks for digging into the failures @pheyos. I'll see if I can get a quick fix in for item 3 Issue with a select name on the single metric viewer page.

@bhavyarm
Copy link
Contributor Author

@peteharverson @1Copenut and I zoomed about 1 and 2. I will log an issue for you. Looks like it needs more discussion. Thanks @pheyos

@peteharverson
Copy link
Contributor

The fix for the 'Interval' select for the anomalies table is up - #154185

@qn895
Copy link
Member

qn895 commented Mar 31, 2023

This PR should now be unblocked after this #154185 was merged in

@bhavyarm
Copy link
Contributor Author

bhavyarm commented Apr 3, 2023

@peteharverson @1Copenut and I zoomed about 1 and 2. I will log an issue for you. Looks like it needs more discussion. Thanks @pheyos

#154294

@kibana-ci
Copy link
Collaborator

kibana-ci commented Jun 21, 2023

💔 Build Failed

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #62 / ml Accessibility for user with full ML access with data loaded anomaly detection create population job validation step
  • [job] [logs] FTR Configs #62 / transform Accessibility "after all" hook: afterTestSuite.trigger in "transform Accessibility"
  • [job] [logs] FTR Configs #62 / transform Accessibility for user with full Transform access "after all" hook in "for user with full Transform access"
  • [job] [logs] FTR Configs #62 / transform Accessibility for user with full Transform access with data loaded "after all" hook: afterTestSuite.trigger for "runs the latest transform and displays management page"
  • [job] [logs] FTR Configs #62 / transform Accessibility for user with full Transform access with data loaded create pivot transform configuration step JSON editor

Metrics [docs]

Unknown metric groups

ESLint disabled line counts

id before after diff
enterpriseSearch 13 15 +2
securitySolution 411 415 +4
total +6

Total ESLint disabled count

id before after diff
enterpriseSearch 14 16 +2
securitySolution 492 496 +4
total +6

History

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

cc @bhavyarm

@bhavyarm
Copy link
Contributor Author

closing this without merging & opening a new pr to unskip to make a clean start of weird test failures on this.

@bhavyarm bhavyarm closed this Jun 21, 2023
@bhavyarm bhavyarm deleted the unskipMLa11yTests branch June 21, 2023 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting :ml Project:Accessibility release_note:skip Skip the PR/issue when compiling release notes v8.8.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants