Fix flaky broken link checker #3173

colinrotherham · 2023-09-25T16:37:17Z

We have a very flaky npm run check-links script at the moment

This PR partially fixes #3113 and alphagov/govuk-frontend#4223 makes the link checker stable by:

~~1. Avoiding sitemap.xml parsing~~
2. Remove flag --source-maps
~~3. Remove flag --recursive (uses glob instead)~~
~~4. Replace reporter tap-mocha-reporter with tap-min~~
5. Saving output to disk before piping to reporter

Task output

In the past we've switched reporter to increase stability but see Munter/hyperlink#179 (comment). The best fix was to pipe to disk first, using the README suggestion in #3173 (comment)

Initially this PR bypassed the reporter entirely but output was too noisy:

> check-links
> hyperlink --canonicalroot https://design-system.service.gov.uk/ --internal --root deploy/public --skip "govuk/all.css" --skip "govuk-frontend/all.css" --skip "govuk-frontend/all.js" --skip "<YOUR-DOMAIN>" $(glob "deploy/public/**/*.html")

TAP version 13
# Crawling internal assets
ok 1 load deploy/public/index.html
ok 2 load deploy/public/google921002f57f264802.html
ok 3 load deploy/public/styles/index.html
ok 4 load deploy/public/sitemap/index.html
ok 5 load deploy/public/privacy-policy/index.html
ok 6 load deploy/public/patterns/index.html
ok 7 load deploy/public/get-started/index.html
ok 8 load deploy/public/get-in-touch/index.html
ok 9 load deploy/public/design-system-team/index.html
ok 10 load deploy/public/cookies/index.html

netlify · 2023-09-25T16:38:22Z

✅ You can preview this change here:

Name	Link
🔨 Latest commit	`f68a012`
🔍 Latest deploy log	https://app.netlify.com/sites/govuk-design-system-preview/deploys/6516f50bb63d880008d6624a
😎 Deploy Preview	https://deploy-preview-3173--govuk-design-system-preview.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

colinrotherham · 2023-09-27T08:02:15Z

Moving back into draft as it's still returning exit codes but without error logs

colinrotherham · 2023-09-28T12:00:48Z

This branch has been run successfully 10x in a row:

package.json

domoscargin · 2023-09-28T15:48:43Z

All passing great locally - coupla questions. The output is annoying, but auditing the errors is just a "search for 'not ok'" away, so really not a biggie imo.

When used without `--internal` we see problems parsing external URLs when the `--source-maps` flag is also enabled Removing it just in case

colinrotherham · 2023-09-28T16:58:10Z

Thanks @domoscargin

I've reduced the PR to remove --source-maps and drop the formatter only

Looking good still:

Task output

…The output is annoying, but auditing the errors is just a "search for 'not ok'" away, so really not a biggie imo.

Definitely frustrating that the fix makes the output worse

That said, just noticed an alternative reporter tap-min was updated last month:

[SPIKE] Fix flaky broken link checker (with reporter) #3182

Could be another option but will run it a few times there and see if it copes

romaricpascal · 2023-09-29T13:24:27Z

It's cheap enough to swap reporters so I'd be keen to try tap-min by merging and monitoring what happens for a bit.

If it fails, there's another option with tap-spot, mentioned in hyperlink's README... and we can always pipe to grep on CI to clean things up a bit if all reporters fail 😊

colinrotherham · 2023-09-29T14:33:49Z

@romaricpascal Thanks, sadly tried using tap-min and we have 2 failures:

For comparison on main I've also seen failures in 2 out of 10 runs:

colinrotherham · 2023-09-29T15:41:44Z

If it fails, there's another option with tap-spot, mentioned in hyperlink's README... and we can always pipe to grep on CI to clean things up a bit if all reporters fail 😊

@romaricpascal Good news. Using tap-min via tee runs perfectly:

I've pushed it up, ready to review again 👍

colinrotherham · 2023-09-29T15:50:07Z

Note: With tap recent updates shall we keep tap-mocha-reporter and/or await tapjs/tap-mocha-reporter#76?

I'll do some more runs with the existing tap-mocha-reporter as Dependabot will update it on Monday anyway

Saving to disk before hitting the reporter appears to remove the flaky non-zero exit codes too Co-authored-by: Romaric Pascal <romaric.pascal@digital.cabinet-office.gov.uk>

romaricpascal · 2023-09-29T16:04:02Z

Whoop! 🥳 If adding tee solves the issue, that's neat. It also opens us up (in the future) to uploading the link checking results as an artifact when it fails, which could help automation/reprocessing things locally, so don't mind the extra command in between.

If things work just as well with tee and tap-mocha-reporter, I'd be keen to stick with tap-mocha-reporter (wider pool of people maintaining the project, being part of the tapjs org).

Thanks for re-running the checks so many times 😊

colinrotherham · 2023-09-29T16:34:40Z

Yeah looking good still:

To confirm, exit codes are still returned on failure too:
https://github.com/alphagov/govuk-design-system/actions/runs/6354399404/job/17260821602

> check-links
> hyperlink --canonicalroot https://design-system.service.gov.uk/ --internal --recursive deploy/public/sitemap.xml | tee check-links.log | tap-mocha-reporter min

Guessing --root from input files: file:///home/runner/work/govuk-design-system/govuk-design-system/deploy/public/

  2227 passing (39s)
  8 pending
  5 failing

  1) load deploy/public/components/hint-colin-broke-this-link:
     load deploy/public/components/hint-colin-broke-this-link
  

  2) load deploy/public/components/error-message-colin-broke-this-link:
     load deploy/public/components/error-message-colin-broke-this-link
  

  3) load deploy/public/components/fieldset-colin-broke-this-link:
     load deploy/public/components/fieldset-colin-broke-this-link
  

  4) load deploy/public/components/label-colin-broke-this-link:
     load deploy/public/components/label-colin-broke-this-link
  

  5) load deploy/public/components/tag-colin-broke-this-link:
     load deploy/public/components/tag-colin-broke-this-link

colinrotherham · 2023-09-29T16:50:17Z

@romaricpascal For info, Windows checks passed too (it includestee which is nice)

EditorConfig check has failures though but I've fixed these in #3183

colinrotherham changed the title ~~Simplify link checker, fix known redirects~~ Fix flaky link checker Sep 25, 2023

colinrotherham force-pushed the check-links-reporter branch from f1f8c1c to 7e9c840 Compare September 25, 2023 16:40

colinrotherham changed the title ~~Fix flaky link checker~~ Fix flaky broken link checker Sep 25, 2023

colinrotherham force-pushed the check-links-reporter branch 2 times, most recently from 24f69c3 to e4b596e Compare September 25, 2023 16:56

colinrotherham marked this pull request as draft September 27, 2023 08:02

colinrotherham force-pushed the check-links-reporter branch from e4b596e to ab51791 Compare September 27, 2023 08:05

colinrotherham force-pushed the check-links-reporter branch from ab51791 to 0e0f7f5 Compare September 28, 2023 12:02

colinrotherham marked this pull request as ready for review September 28, 2023 12:05

colinrotherham mentioned this pull request Sep 28, 2023

Improve our broken link checking tests #3113

Open

6 tasks

domoscargin reviewed Sep 28, 2023

View reviewed changes

package.json Outdated Show resolved Hide resolved

domoscargin reviewed Sep 28, 2023

View reviewed changes

package.json Outdated Show resolved Hide resolved

domoscargin reviewed Sep 28, 2023

View reviewed changes

package.json Outdated Show resolved Hide resolved

Remove check-links flag --source-maps

715df2c

When used without `--internal` we see problems parsing external URLs when the `--source-maps` flag is also enabled Removing it just in case

colinrotherham force-pushed the check-links-reporter branch from 0e0f7f5 to 7d3a9ab Compare September 28, 2023 16:11

colinrotherham mentioned this pull request Sep 28, 2023

[SPIKE] Fix flaky broken link checker (with reporter) #3182

Closed

Save check-links output to check-links.log via tee

f68a012

Saving to disk before hitting the reporter appears to remove the flaky non-zero exit codes too Co-authored-by: Romaric Pascal <romaric.pascal@digital.cabinet-office.gov.uk>

colinrotherham force-pushed the check-links-reporter branch from 1ff4d02 to f68a012 Compare September 29, 2023 16:02

romaricpascal approved these changes Sep 29, 2023

View reviewed changes

colinrotherham mentioned this pull request Sep 29, 2023

Fix flaky broken link checker alphagov/govuk-frontend-docs#350

Merged

colinrotherham merged commit 5918669 into main Sep 29, 2023

colinrotherham deleted the check-links-reporter branch September 29, 2023 16:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix flaky broken link checker #3173

Fix flaky broken link checker #3173

colinrotherham commented Sep 25, 2023 •

edited

Loading

netlify bot commented Sep 25, 2023 •

edited

Loading

colinrotherham commented Sep 27, 2023

colinrotherham commented Sep 28, 2023

domoscargin commented Sep 28, 2023

colinrotherham commented Sep 28, 2023 •

edited

Loading

romaricpascal commented Sep 29, 2023

colinrotherham commented Sep 29, 2023 •

edited

Loading

colinrotherham commented Sep 29, 2023

colinrotherham commented Sep 29, 2023 •

edited

Loading

romaricpascal commented Sep 29, 2023 •

edited

Loading

colinrotherham commented Sep 29, 2023

colinrotherham commented Sep 29, 2023 •

edited

Loading

Fix flaky broken link checker #3173

Fix flaky broken link checker #3173

Conversation

colinrotherham commented Sep 25, 2023 • edited Loading

Task output

netlify bot commented Sep 25, 2023 • edited Loading

✅ You can preview this change here:

colinrotherham commented Sep 27, 2023

colinrotherham commented Sep 28, 2023

domoscargin commented Sep 28, 2023

colinrotherham commented Sep 28, 2023 • edited Loading

Task output

romaricpascal commented Sep 29, 2023

colinrotherham commented Sep 29, 2023 • edited Loading

colinrotherham commented Sep 29, 2023

colinrotherham commented Sep 29, 2023 • edited Loading

romaricpascal commented Sep 29, 2023 • edited Loading

colinrotherham commented Sep 29, 2023

colinrotherham commented Sep 29, 2023 • edited Loading

colinrotherham commented Sep 25, 2023 •

edited

Loading

netlify bot commented Sep 25, 2023 •

edited

Loading

colinrotherham commented Sep 28, 2023 •

edited

Loading

colinrotherham commented Sep 29, 2023 •

edited

Loading

colinrotherham commented Sep 29, 2023 •

edited

Loading

romaricpascal commented Sep 29, 2023 •

edited

Loading

colinrotherham commented Sep 29, 2023 •

edited

Loading