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: Automatically update coverage and quality report on merge #2122

Merged
merged 5 commits into from
Feb 20, 2024

Conversation

nschonni
Copy link
Contributor

@nschonni nschonni commented Nov 9, 2021

Closes #2061


WAI Preview Link (Last built on Tue, 13 Feb 2024 19:23:17 GMT).

@nschonni
Copy link
Contributor Author

nschonni commented Nov 9, 2021

@jongund I think you originally set this up, so hope this makes sense

"git add examples/index.html"
"git add examples/index.html",
"npm run coverage-report",
"git add coverage/"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@howard-e are these explicit git add still ok with the new lint-staged?

Copy link
Contributor

Choose a reason for hiding this comment

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

Those should be fine

@a11ydoer
Copy link
Contributor

@evmiguel will faciliate the communication.

@mcking65
Copy link
Contributor

mcking65 commented Oct 1, 2023

I've not looked at this PR for a long time; somehow I assumed that the coverage and quality report was already getting auto-updated with merges to main. Now, I'm not sure that it is. If not, we need to update this PR.

The coverage and quality report is now part of the APG. It is published in the about section on the page Coverage and Quality Reports | APG | WAI | W3C, which comes from content/about/coverage-and-quality/coverage-and-quality-report.html.

scripts/coverage-report.js is already updated to generate the file to the correct location.

I'd like one more change to the generated content. That is to add the date the reports were last generated under the heading "About These Reports". The first paragraph under the heading could be:

These reports were updated on monthName, dayNumber, yearNumber.

@mcking65
Copy link
Contributor

mcking65 commented Oct 1, 2023

@nschonni @howard-e

I think I have this PR updated. However, I don't understand the point of this check that is failing:

Ensure no git changes 0s

If this check testing to see if a PR would change the report?

I'd assume that that many if not most PRs will change the report. That is the reason for generating a new report on each commit to main.

@howard-e
Copy link
Contributor

howard-e commented Nov 6, 2023

Ensure no git changes 0s

If this check testing to see if a PR would change the report?

I'd assume that that many if not most PRs will change the report. That is the reason for generating a new report on each commit to main.

@mcking65 I'm unsure of the historical need/approach for the job, but the answer seems to be yes and that assumption is correct, just going through this. The lint-staged + husky hook should be adding any coverage report file changes to the contributor's commits at the point they do a git commit, so there isn't any new changes during the PR workflows being ran.

The changes should already be included at the point the git push is done.

However, I don't understand the point of this check that is failing

The hook works if any file changes have been made to content/patterns/**/examples/*.html which this PR doesn't. And it's been some time since the report has been updated, so it's outdated.

Manually running npm run coverage-report (I get the same reported changes when I do that) and pushing, in this instance should be fine. Future updates to any of the content/patterns/**/examples/*.html files should use the hook as expected (assuming the hook isn't bypassed when any of those relevant files are updated).

Also, these changes are good to me, after pushing those updated coverage files.

@nschonni
Copy link
Contributor Author

nschonni commented Nov 6, 2023

Gave this a rebase and regenerated the files by running the coverage-report script

Copy link
Contributor

@howard-e howard-e left a comment

Choose a reason for hiding this comment

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

@nschonni thanks. Looks good to me!

@howard-e howard-e requested a review from mcking65 November 7, 2023 14:34
@ccanash
Copy link

ccanash commented Jan 30, 2024

Hi @mcking65 flagging this for your review

@css-meeting-bot
Copy link
Member

The ARIA Authoring Practices (APG) Task Force just discussed Auto-updating of coverage and quality report.

The full IRC log of that discussion <jugglinmike> Topic: Auto-updating of coverage and quality report
<Jem> https://github.com//pull/2122
<siri> shirsha
<jugglinmike> github: https://github.com//pull/2122
<Jem> https://www.w3.org/WAI/ARIA/apg/about/coverage-and-quality/
<jugglinmike> Matt_King: There's a preview here. If you go to the "about" page and go down to the "coverage and quality"
<jugglinmike> Matt_King: There isn't anything on this page which indicates when the report was last run
<jugglinmike> Matt_King: It seems to me like the script which does the update should also modify the content to reflect the date (we don't need the time)
<jugglinmike> Matt_King: I'm not sure where it should appear on the page, exactly
<jugglinmike> Jem: That sounds like a good idea--it's a good service to the user
<jugglinmike> Jem: But you're only talking about the "coverage and quality" report, not the entire website, right?
<jugglinmike> Matt_King: That's right
<jugglinmike> Matt_King: We don't necessarily have to do it as part of this pull request
<jugglinmike> Matt_King: But without it, there's no way to tell that it's working just from looking at the website. To me, that decreases its utility
<jugglinmike> Jem: Maybe just below the heading--"Last updated on ..."
<jugglinmike> howard-e: This script runs on every workflow push--not just commits to the "main" branch. That means it also operates on pull request branches
<jugglinmike> Matt_King: We already have a solution for "page last updated" on the pattern example pages
<jugglinmike> Matt_King: It's displayed at the bottom of the page. Is that fixed-position?
<jugglinmike> Jem: No, it is statically positioned at the very bottom
<jugglinmike> Matt_King: How is it that the example pages have that addition section at the bottom, and the other pages do not? Is it possible to pull that extra section in to this "coverage and quality" report page
<jugglinmike> howard-e: I'd like to think it's possible, of course, but I will need to review the build process to say for sure
<jugglinmike> howard-e: We probably wouldn't want to take that approach unless we wanted to include the "last modified" date for *all* pages in the "About" section
<jugglinmike> Matt_King: Okay! For some future change, we should consider including a "last modified" date on every page in the APG
<jugglinmike> howard-e: The associated CSS will ideally be declared inline within the HTML since this is a template we're editing
<jugglinmike> arigilmore: I can take a shot at this if someone can give me a little direction
<Jem> https://github.com/w3c/aria-practices/blob/main/content/about/coverage-and-quality/coverage-and-quality-report.html
<jugglinmike> [the group reviews the project's file organization and identifies the two files which need to be modified]
<Jem> https://github.com/w3c/aria-practices/blob/main/scripts/coverage-report.template

@ariellalgilmore
Copy link
Contributor

I updated the coverage-and-quality template and script. The deploy preview won't show the Page last updated: ... until the PR gets merged, which will then run the coverage-and-quality script

@mcking65
Copy link
Contributor

I manually ran the report and pushed the change so people can preview it with the new update date functionality.

Preview the coverage and quality report page

@howard-e
Copy link
Contributor

#2932 has been merged into main to address the link-checker build error.

@mcking65 mcking65 merged commit 16dbe83 into w3c:main Feb 20, 2024
12 of 13 checks passed
@nschonni nschonni deleted the coverage-report branch February 20, 2024 17:36
@@ -17,6 +17,7 @@
<body>
<main>
<h1>Coverage and Quality Reports</h1>
<p>Page last updated: </p>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As shown on the landed commit, adding a stamp will continually fail the diff, except for things landed on the same day as the last time this was updated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add git hook and CI for updating the coverage and quality report page
8 participants