test: check links

[casperdcl]

• script to search for and check for problematic (dead) links
  • check absolute links
  • check relative markdown links
  • define exclusions list
  • deal with links that have brackets ()
    • might need recursive/balancing groups regex (https://pypi.org/project/regex/)?
  • should warnings (status codes 0xx, 3xx etc) be counted as errors instead (see test: check links #958 (comment))?
  • concurrent for speed
  • hooks only for diffs for speed
• python pre-commit hooks
  • config.yml (for local repo)
    • maybe remove in favour of husky (see below)?
  • hooks.yml (for other repos to use)
    • document developer usage of pre-commit hooks?
• yarn tests
  • add yarn.scripts.link-check
  • add yarn.scripts.link-check-diff
  • add husky.hooks.pre-commit
    • remove due to potential windows incompatibility (see below)
  • include .github/ in allthis yarn.scripts checks
• CI
  • only for the diff (otherwise takes ~60s to run)
  • add full (60s) run in a daily cron job?
    • run: yarn link-check
• rename link-check everywhere for consistency
• do something for Windows users
  • remove from pre-commit hooks (leave as CI-only)?
  • add .huskyrc.js checking for OS compatibility?
  • convert bash to py?
  • convert bash to js?
• open a different issue to fix current link errors (fix broken links #974)
• fixes ci: test to check all links #652
• rebased from and closes Script to check links on .md files #690 (which GitHub won't let me re-open)

[casperdcl]

@shcheklein I presume sorting out the current errors is for a different PR? See the output below:

dvc.org (check-links)$ pre-commit run --all-files | grep -v OK
Dead URL Checker.........................................................Failed
- hook id: dead-url
- exit code: 3

public/static/docs/understanding-dvc/related-technologies.md:
 WARNING:301:http://studio.ml/
public/static/docs/use-cases/data-registries.md:
 ERROR:406:http://millionsongdataset.com/pages/getting-dataset/#subset
public/static/docs/user-guide/contributing/docs.md:
 WARNING:000:http://localhost:3000/

 ERROR:404:https://marketplace.visualstudio.com/items?itemName=stkb.rewrap

 WARNING:000:https://nexyjs.org/
public/static/docs/command-reference/pipeline/show.md:
 ERROR:404:https://en.wikipedia.org/wiki/Less_(Unix

 ERROR:404:https://en.wikipedia.org/wiki/More_(command
ERROR:3 failures
---
public/static/docs/user-guide/running-dvc-on-windows.md:
 ERROR:404:https://docs.microsoft.com/en-us/previous-versions/windows/it-pro/windows-server-2003/cc778996(v=ws.10

 ERROR:404:https://en.wikipedia.org/wiki/Less_(Unix
ERROR:1 failures
---
public/static/docs/install/macos.md:
 WARNING:000:https://support.apple.com/en-us/HT207700
public/static/docs/command-reference/remote/index.md:
 WARNING:000:https://object-storage.example.com
README.md:
 ERROR:404:https://circleci.com/gh/iterative/dvc.org
public/static/docs/user-guide/privacy.md:
 ERROR:400:https://accounts.google.com/o/oauth2/auth
ERROR:2 failures
---
public/static/docs/command-reference/remote/modify.md:
 WARNING:000:https://myendpoint.com

 WARNING:000:https://object-storage.example.com
public/static/docs/command-reference/diff.md:
 ERROR:404:https://remote.dvc.org/get-started
ERROR:1 failures
---
public/static/docs/understanding-dvc/collaboration-issues.md:
 ERROR:404:https://en.wikipedia.org/wiki/Hyperparameter_(machine_learning
public/static/docs/command-reference/remote/add.md:
 ERROR:404:https://drive.google.com/drive/folders/0AIac4JZqHhKmUk9PDA

 WARNING:000:https://object-storage.example.com
ERROR:2 failures
---
public/static/docs/user-guide/contributing/core.md:
 WARNING:000:http://127.0.0.1:10000/devstoreaccount1;
public/static/docs/understanding-dvc/resources.md:
 ERROR:404:https://www.kaggle.com/rtatman/kerneld4769833fe
ERROR:1 failures
---

[shcheklein]

in term of the pre-commit hooks - we already use husky (?) and something is being installed with yarn. So we would probably need to plug into it.

[casperdcl]

@shcheklein I don't quite follow - yarn is being used to run scripts https://github.com/iterative/dvc.org/blob/92421404b05e73e9b3e130299495b4a703ffe080/package.json#L6-L16

While we could add check-links there I don't think we should use it as it would check all files (takes about a minute).

For checking patches, I thought we should use whatever the main repo does (currently pre-commit) surely?

[shcheklein]

@casperdcl when you just run yarn for the first time it install the dependencies and the pre-commit hook (which runs linters, prettier, etc, etc).

[shcheklein]

Good stuff! Please check a few comments. Especially, not clear how does it integrate with the existing hooks.

[shcheklein]

looks great! let me know when we are ready to merge it

[casperdcl]

@shcheklein there's just

should warnings (status codes 0xx, 3xx etc) be counted as errors instead (see #958 (comment))?

It's probably not that relevant since the 000/301 (as well as 404) etc should be resolved in a different issue later. So yes, good to merge.

[casperdcl]

good to merge now

• complete overhaul of circleci included in order to use daily cron jobs
• .pre-commit-hooks.yaml included for others to use but not actually used by us (yet) due to potential lack of windows support
• yarn/husky pre-commit hook also not used for same reason
• problematic links to be fixed in different issue/PR

[shcheklein]

thanks, @casperdcl !

[jorgeorpinel]

Oh so it's actually matching the $ character for cases like below?

https://github.com/iterative/dvc.org/blob/2d99864409fde25a06d6d2378fc6e24aa41cc4ac/src/components/DownloadButton/index.js#L33-L48

[casperdcl]

Yes my regexes match only up to the literal $. It seems a good compromise between blacklisting false negatives and programming false negatives detection.

[jorgeorpinel]

Very neat! Should we briefly document how this thing works somewhere? Is there a follow up PR or issues on the few unchecked boxes from the description above? Thanks

[shcheklein]

I think decisions were made, links were fixed already (or added to exclusions) ... not sure we need to document it tbh. Quite internal stuff, similar to redirects. May be some message to the script can be added that explains how to change the files if needed. Again not a top priority.

[jorgeorpinel]

I have one more Q @shcheklein I see you added some excluded URLs in ba42034...32f63df#diff-01c133d99b04d1390925d3d06f3bbe77 but they are valid. Specifically:

http://millionsongdataset.com/pages/getting-dataset/#subset
https://circleci.com/gh/iterative/dvc.org
https://marketplace.visualstudio.com/items?itemName=stkb.rewrap
https://www.kaggle.com/rtatman/kerneld4769833fe

Why?

First one probably because of non secure HTTP?

---

In general I was just a bit worried that it would be tricky to maintain this list manually. Remembering to add or remove sample links so they're not checked. But I guess if you don't add it the CI check will fail, and if you don't remove it, nothing bad will happen.

[shcheklein]

@jorgeorpinel some of them do not work from cli and work from the browser. for some of them I would check the script once again.

[casperdcl]

Yes for example circleci does weird non-standard redirection

[jorgeorpinel]

Got it.

p.s. yarn link-check doesn't work on Mac: pcregrep: command not found

UPDATE: Got pcregrep with brew install pcre but now I'm getting a bunch of sed: illegal option -- r

UPDATE's UPDATE: Got GNU sed with brew install gnu-sed (and added it to my PATH)

[casperdcl]

Yes that's one of the reasons why this is a CI-only check by default rather than a pre-commit hook... no guarantee that it'll run on dev machines