Infrastructure: Use markdown to format PR comments containing regression test coverage reports #1562

spectranaut · 2020-10-09T16:06:18Z

Now that we have a github action to add a comment with the regression coverage report we can take advantage of markdown formatting and format the report appropriately.

github-actions · 2020-10-09T16:06:58Z

#### Regression test coverage:


#### Examples without any regression tests:

- button/button_idl.html
- dialog-modal/alertdialog.html


#### Examples missing some regression tests:

- combobox/combobox-autocomplete-both.html:
   - combobox-id
- combobox/combobox-autocomplete-list.html:
   - combobox-id
- combobox/combobox-autocomplete-none.html:
   - combobox-id
- combobox/grid-combo.html:
   - textbox-key-down-arrow
   - textbox-key-up-arrow
- dialog-modal/datepicker-dialog.html:
   - textbox-aria-describedby
- menu-button/menu-button-actions-active-descendant.html:
   - menu-up-arrow
   - menu-down-arrow
   - menu-character
- spinbutton/datepicker-spinbuttons.html:
   - spinbutton-down-arrow
   - spinbutton-up-arrow
   - spinbutton-page-down
   - spinbutton-page-up
   - spinbutton-home
   - spinbutton-end
- 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-aria-controls
   - toolbar-button-aria-pressed
   - toolbar-button-aria-hidden
   - toolbar-radiogroup-role
   - toolbar-radiogroup-aria-label
   - toolbar-radio-role
   - toolbar-radio-aria-checked
   - toolbar-radio-aria-hidden
   - toolbar-button-aria-disabled
   - toolbar-menubutton-aria-label
   - toolbar-menubutton-aria-haspopup
   - toolbar-menubutton-aria-controls
   - toolbar-menubutton-aria-expanded
   - toolbar-menu-role
   - toolbar-menu-aria-label
   - toolbar-menuitemradio-role
   - toolbar-menuitemradio-aria-checked
   - toolbar-menuitemradio-tabindex
   - toolbar-spinbutton-role
   - toolbar-spinbutton-aria-label
   - toolbar-spinbutton-aria-valuenow
   - toolbar-spinbutton-aria-valuetext
   - toolbar-spinbutton-aria-valuemin
   - toolbar-spinbutton-aria-valuemax


#### 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.
  2 example pages have no regression tests.
  8 example pages are missing approximately 61 out of approximately 775 tests.

ERROR - missing tests:

  Please write missing tests for this report to pass.

spectranaut · 2020-10-09T16:08:12Z

Whoops well that didn't work -- will have to investigate!

nschonni · 2020-10-09T16:16:44Z

.github/workflows/coverage-report.yml

@@ -29,7 +29,7 @@ jobs:
          github-token: ${{secrets.GITHUB_TOKEN}}
          script: |
            const fs = require('fs');
-            const commentBody = '```' + fs.readFileSync('coverage.log', 'utf8') + '```';
+            const commentBody = fs.readFileSync('coverage.log', 'utf8');


I think the issue you're running into is that this change won't show up till this PR lands, because it is using pull_request_target

The code fence was just my hacky way of dealing with the whitespace collapsing on the original format, but your other changes should address that

nschonni · 2020-10-09T16:32:36Z

.github/workflows/coverage-report.yml

@@ -1,7 +1,7 @@
 name: Regression Tests Coverage Report

 on:
-  pull_request_target:
+  pull_request:


Without the pull_request_target, I don't think it has access to the Secret needed to post/update the comment

I see, of course, oh well -- I'll drop that commit.

If you want to test it, you can push the updated Yaml to your own master branch in your fork (or change your default branch to this branch) and open a PR on your repo. You'd need to to add your own PAT as a GitHub secret too
Actual steps I needed do:

Setup the GH_TOKEN secret on https://github.com/nschonni/aria-practices/settings/secrets, but I actually already had it from the gh-pages deploy testin

Pushed your branch to my fork

Pushed a commit, with just the removal of the code fence to my default branch nschonni@cb9183c

Open PR from your branch to my default, so it would include your fixed up rendering in the script, but triggered the workflow in main.

Profit Switch the regression report to use markdown nschonni/aria-practices#40 (comment)

None of that is required for changes here, just if you want to iterate on the styling on your own fork

Thanks so much for detailing I'm sure I'll want to test the actions like this in the future!

Or if you don't want to over-think it, like apparently did, just edit the bot comment after it's created/updated and remove the backticks manually 😆

hahaha omg why didn't we think of that 👀

nschonni · 2020-10-09T21:17:01Z

@spectranaut here's how it looks nschonni#40 (comment)
I pushed the fix to my default branch for the workflow, then submitted your PR on top

nschonni · 2020-10-09T21:18:02Z

test/util/report.js

 console.log(missingTestIdsReport || 'None found.\n');

-console.log('SUMMARTY:\n');
+console.log('### SUMMARTY:\n');
 console.log('  ' + exampleFiles.length + ' example pages found.');


The whitspace padding gets lost, so it can probably get dropped here and the lines below. Not sure if \t would work instead if the indent should be kept

That white space padding is not super important, it was just ot make the console output more readable and less of a wall of text. In this context I think it's totally fine!

Random though, you could just make these 2 items a list

nschonni · 2020-10-10T14:39:01Z

Pulling the comment from @carmacleod from my test PR nschonni#40 (comment)

Typo: "SUMMARTY:"

Also, not quite sure what this heading means?
"Examples documentation table rows without data-test-ids:"

Is it "Examples documenting table rows without data-test-ids:"
or "Example documentation table rows without data-test-ids:"
or simply "Examples without data-test-ids:"?

mcking65

Love it! Some very simple changes to make.

mcking65 · 2020-10-21T20:18:30Z

test/util/report.js

@@ -260,14 +260,15 @@ fs.readdirSync(testsPath).forEach(function (testFile) {
  }
 });

-console.log('\nExamples without any regression tests:\n');
+console.log('\n## Regression test coverage:\n');
+console.log('\n### Examples without any regression tests:\n');


Please make level 4 headings for all the headings. The GitHub comments themselves have level 3 heading, and we want these to be subheadings.

mcking65 · 2020-10-21T20:20:06Z

test/util/report.js

 console.log(missingTestIdsReport || 'None found.\n');

-console.log('SUMMARTY:\n');


Fix spelling of summary

spectranaut · 2020-10-22T20:44:20Z

Fixed! Thanks of the feedback everyone.

@carmacleod -- I tried to be a bit more clear, but the "Keyboard support" table and the "Role, Property, State, and Tabindex Attributes" table should have have "data-test-id" attributes on the tr -- this is used to link the regression tests to the specific documentation in the example page. So if there is a row that doesn't have a "data-test-id" it shows up here!

test/util/report.js

carmacleod

+1

Thanks @nschonni and @spectranaut !

nschonni · 2020-10-22T21:29:59Z

Someone with permissions on this repo could edit the bot comment above to remove the backticks if you want to see what the end result will look like

test/util/report.js

mcking65

Awesome!

Switch the regression report to use markdown

85b8ebe

spectranaut requested a review from mcking65 October 9, 2020 16:06

nschonni reviewed Oct 9, 2020

View reviewed changes

spectranaut force-pushed the markdown-regression-report branch from 03f74df to 85b8ebe Compare October 9, 2020 16:43

nschonni reviewed Oct 9, 2020

View reviewed changes

mcking65 requested changes Oct 21, 2020

View reviewed changes

Change headings for clarify

9874e14

Merge branch 'master' into markdown-regression-report

bc788a5

carmacleod reviewed Oct 22, 2020

View reviewed changes

test/util/report.js Outdated Show resolved Hide resolved

carmacleod approved these changes Oct 22, 2020

View reviewed changes

Fix grammar

6c65e25

carmacleod reviewed Oct 22, 2020

View reviewed changes

test/util/report.js Outdated Show resolved Hide resolved

Grammar

ef6e15f

mcking65 approved these changes Oct 22, 2020

View reviewed changes

Merge branch 'master' into markdown-regression-report

4dc9956

mcking65 merged commit c4d8d14 into master Oct 22, 2020

mcking65 deleted the markdown-regression-report branch October 22, 2020 23:42

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 Oct 22, 2020

mcking65 added this to the 1.2 Release 1 milestone Oct 22, 2020

mcking65 changed the title ~~Switch the regression report to use markdown in comment~~ Infrastructure: Use markdown to format PR comments containing regression test coverage reports Oct 22, 2020

Clemens-git76 mentioned this pull request Apr 16, 2024

[Snyk] Upgrade node-fetch from 2.7.0 to 3.3.2 Clemens-git76/aria-practices#4

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Infrastructure: Use markdown to format PR comments containing regression test coverage reports #1562

Infrastructure: Use markdown to format PR comments containing regression test coverage reports #1562

spectranaut commented Oct 9, 2020

github-actions bot commented Oct 9, 2020 •

edited

Loading

spectranaut commented Oct 9, 2020

nschonni Oct 9, 2020

nschonni Oct 9, 2020

nschonni Oct 9, 2020

spectranaut Oct 9, 2020

nschonni Oct 9, 2020 •

edited

Loading

spectranaut Oct 10, 2020

nschonni Oct 10, 2020 •

edited

Loading

spectranaut Oct 22, 2020

nschonni commented Oct 9, 2020

nschonni Oct 9, 2020

spectranaut Oct 10, 2020

nschonni Oct 22, 2020

nschonni commented Oct 10, 2020

mcking65 left a comment

mcking65 Oct 21, 2020 •

edited

Loading

mcking65 Oct 21, 2020

spectranaut commented Oct 22, 2020

carmacleod left a comment

nschonni commented Oct 22, 2020

mcking65 left a comment

		console.log(missingTestIdsReport \|\| 'None found.\n');

		console.log('SUMMARTY:\n');

Infrastructure: Use markdown to format PR comments containing regression test coverage reports #1562

Infrastructure: Use markdown to format PR comments containing regression test coverage reports #1562

Conversation

spectranaut commented Oct 9, 2020

github-actions bot commented Oct 9, 2020 • edited Loading

spectranaut commented Oct 9, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nschonni Oct 9, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nschonni Oct 10, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nschonni commented Oct 9, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nschonni commented Oct 10, 2020

mcking65 left a comment

Choose a reason for hiding this comment

mcking65 Oct 21, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spectranaut commented Oct 22, 2020

carmacleod left a comment

Choose a reason for hiding this comment

nschonni commented Oct 22, 2020

mcking65 left a comment

Choose a reason for hiding this comment

github-actions bot commented Oct 9, 2020 •

edited

Loading

nschonni Oct 9, 2020 •

edited

Loading

nschonni Oct 10, 2020 •

edited

Loading

mcking65 Oct 21, 2020 •

edited

Loading