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

Infrastructure: Fix previously excluded ESLint issues #1610

Merged
merged 25 commits into from
Jan 8, 2021

Conversation

nschonni
Copy link
Contributor

@nschonni nschonni commented Nov 11, 2020

Built off of #1602, then when through and addressed the missing/undefined variable issues by example

@github-actions
Copy link
Contributor

Regression test coverage:

Examples without any regression tests:

  • dialog-modal/alertdialog.html

Examples missing some regression tests:

  • dialog-modal/datepicker-dialog.html:
    • textbox-aria-describedby
  • menu-button/menu-button-actions-active-descendant.html:
    • menu-up-arrow
    • menu-down-arrow
    • menu-character
  • toolbar/toolbar.html:
    • toolbar-tab
    • toolbar-right-arrow
    • toolbar-left-arrow
    • toolbar-home
    • toolbar-end
    • toolbar-toggle-esc
    • toolbar-toggle-enter-or-space
    • toolbar-radio-enter-or-space
    • toolbar-radio-down-arrow
    • toolbar-radio-up-arrow
    • toolbar-button-enter-or-space
    • toolbar-menubutton-enter-or-space-or-down-or-up
    • toolbar-menu-enter-or-space
    • toolbar-menu-down-arrow
    • toolbar-menu-up-arrow
    • toolbar-menu-escape
    • toolbar-spinbutton-down-arrow
    • toolbar-spinbutton-up-arrow
    • toolbar-spinbutton-page-down
    • toolbar-spinbutton-page-up
    • toolbar-checkbox-space
    • toolbar-link-enter-or-space
    • toolbar-spinbutton-role

Example pages with Keyboard or Attribute table rows that do not have data-test-ids:

  • dialog-modal/alertdialog.html
    • "Keyboard Support" table(s):
      • Tab
      • Shift + Tab
      • Escape
      • Command + S
      • Control + S
    • "Attributes" table(s):
      • alertdialog
      • aria-labelledby=IDREF
      • aria-describedby=IDREF
      • aria-modal=true
      • alert

SUMMARY:

55 example pages found.
1 example pages have no regression tests.
3 example pages are missing approximately 27 out of approximately 780 tests.

ERROR - missing tests:

Please write missing tests for this report to pass.

@nschonni
Copy link
Contributor Author

@jongund even if this doesn't land, there might be a few bits to apply to your in-progress PRs

@nschonni nschonni changed the title chore: Remove passing no-console Infrastrucutre: Fix remaining ESLint issues Nov 11, 2020
Copy link
Contributor

@spectranaut spectranaut left a comment

Choose a reason for hiding this comment

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

I'm in favor of this change and it all looks good, thanks so much to getting to this work, @nschonni. Just curious, did you use a tool to clean these up?

@nschonni
Copy link
Contributor Author

I don't think much of this was auto-fixable. I think I probably just manually cleaned it up while I was trying to distract my brain

@spectranaut
Copy link
Contributor

@nschonni If there was a tool I wanted to know about it :) I really appreciate the work!

@mcking65
Copy link
Contributor

mcking65 commented Dec 1, 2020

@nschonni
Sorry, think #1623 resulted in conflicts. When you get a chance, could you please rebase?

@mcking65 mcking65 changed the title Infrastrucutre: Fix remaining ESLint issues Infrastructure: Fix previously excluded ESLint issues Dec 1, 2020
@mcking65 mcking65 added enhancement Any addition or improvement that doesn't fix a code bug or prose inaccuracy Infrastructure Related to maintaining task force and repo operations, processes, systems, documentation labels Dec 1, 2020
@mcking65 mcking65 added this to the 1.2 Release 1 milestone Dec 1, 2020
@nschonni
Copy link
Contributor Author

nschonni commented Dec 1, 2020

@mcking65 no problem, it was based on that other PR, but I guess got out of sync. I've rebased this one now

@mcking65
Copy link
Contributor

@nschonni
I should have merged right after you last rebased ... sorry. So, I updated the branch and surprised to see some failing tests.

@nschonni nschonni force-pushed the test-linting branch 2 times, most recently from 9b172f2 to 9ff0a93 Compare December 13, 2020 19:04
@mcking65 mcking65 merged commit 0f03626 into w3c:master Jan 8, 2021
@nschonni nschonni deleted the test-linting branch January 8, 2021 20:53
@mcking65
Copy link
Contributor

mcking65 commented Jan 8, 2021

@nschonni
Just after merging this, regression started failing again for the spinbutton increment/decrement tests in #1712. I don't understand how that is possible since that testing passed for this PR. Maybe unrelated? Maybe related? Confused.

@nschonni
Copy link
Contributor Author

nschonni commented Jan 8, 2021

Not related to this, I think it's hitting another issue related to the new year. EX: it also failed on PRs yesterday https://github.com/w3c/aria-practices/runs/1661714101?check_suite_focus=true

@nschonni
Copy link
Contributor Author

nschonni commented Jan 8, 2021

I fixed it in e4e0cf7 as part of the larger #1640, but I can split that out if you want a quicker PR to get stuff green again

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Any addition or improvement that doesn't fix a code bug or prose inaccuracy Infrastructure Related to maintaining task force and repo operations, processes, systems, documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants