add link_checker

[priyanshuone6]

Description

Added workflow to test urls in the docs
Referred this

Testing

Tested this workflow locally using nektos/act

Fixes #719

Type of change

Please add a line in the relevant section of CHANGELOG.md to document the change (include PR #) - note reverse order of PR #s. If necessary, also add to the list of breaking changes.

• New feature (non-breaking change which adds functionality)
• Optimization (back-end change that speeds up the code)
• Bug fix (non-breaking change which fixes an issue)

Key checklist:

• No style issues: $ flake8
• All tests pass: $ python run-tests.py --unit
• The documentation builds: $ cd docs and then $ make clean; make html

You can run all three at once, using $ python run-tests.py --quick.

Further checks:

• Code is commented, particularly in hard-to-understand areas
• Tests added that prove fix is effective or that feature works

[codecov]

Codecov Report

Merging #1347 (b789f5b) into develop (f8b58f2) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop    #1347   +/-   ##
========================================
  Coverage    98.15%   98.15%           
========================================
  Files          272      272           
  Lines        15577    15577           
========================================
  Hits         15289    15289           
  Misses         288      288           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f8b58f2...389893c. Read the comment docs.

[valentinsulzer]

Thanks @priyanshuone6 ! Did you run this locally (adding a known broken link) to check whether it does pick up broken links? I am slightly surprised that there aren't any broken links, though of course it's possible that we got lucky

[valentinsulzer]

these two options seem contradictory?

[valentinsulzer]

Nevermind, I read the link you shared now. Can you leave quiet mode off so that we can check each file has been checked?

[priyanshuone6]

You are right, they both are quite contradictory, I'll remove the quiet mode.

[priyanshuone6]

I tried to test this workflow locally by using nketos/act and found that when I try to pass more than one extension in file-extension, in our case it's .rst and .md, it stops checking links and instead passes them as all correct.

[priyanshuone6]

I tried using https://github.com/urlstechie/urlchecker-action and found it to be better. It works with different file formats like .py, .rst, .md and it differentiates valid and invalid links using green and red.

[valentinsulzer]

Ok great. Would be good to also test the notebooks (.ipynb)

[tlestang]

Thanks @priyanshuone6 , I tried that workflow on my fork and I noticed the output from url_checker doesn't display colors, see https://github.com/tlestang/PyBaMM/runs/1782502527?check_suite_focus=true
This makes it impossible to separate valid links from broken links.

Another thing is that urls embedded links in SVG inside notebooks are picked up, but with a trailing backslash.
Example

/github/workspace/examples/notebooks/solution-data-and-processed-variables.ipynb 
 --------------------------------------------------------------------------------
https://matplotlib.org/
http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd\
http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd\
http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd\
http://www.w3.org/1999/xlink\
http://www.w3.org/1999/02/22-rdf-syntax-ns#\

Could you maybe specify the exclude_patterns option to exclude such links for the search?

[priyanshuone6]

@tlestang I searched about it and found that github actions do not currently support colors, but in the end, we do get all the invalid links together, look at line 1126
I will look into the embedded links in SVG and try to fix them using exclude pattern.

[valentinsulzer]

This seems to be working now with a manageable number of broken links reported. Weirdly, some of the broken links seem to work fine, so I don't know why URL-checker is reporting they are broken. Let me know if you need help finding working links to replace the broken ones with

[valentinsulzer]

You can ignore the failing benchmarks test. @tlestang can we exclude that test when it comes from a fork?

[priyanshuone6]

Some urls starting with https://github.com/pybamm-team/PyBaMM/pull/ in CHANGELOG.md are being marked as invalid, I tested with different timeout settings to mark as many links valid as it can, and found 10 seconds to be perfect in terms of time and number of links.
Should I include https://github.com/pybamm-team/PyBaMM/pull/ in exclude_patterns or maybe exclude the whole CHANGELOG.md?
Also, should I remove the option to run this workflow in pull requests because every time someone will make a PR, and if it fails due to these unavoidable reasons, a red cross will appear in the PR due to that and it may confuse others?

[valentinsulzer]

Yes, perhaps the way to go is to only run the url check on the cron job. The odds of someone adding a broken link are very low, what usually happens is a link changes and we don't update it because (until now) there was no automatic way of checking all the links. That way, you can also increase the timeout and it doesn't matter if the cron job takes a while. I wouldn't be totally against excluding CHANGELOG.md either since it is usually pretty easy to see if those links are broken.

However, there are some links here that are actually broken and need to be fixed before this branch can be merged. Most of them are from changes to pybamm branch names or file structure and it should be fairly easy to find where the file is now (e.g. control+p in VSCode)

[priyanshuone6]

@tinosulzer Could you please help me with these broken links
https://pybamm.readthedocs.io/en/latest/source/models/lead_acid/composite.html
https://github.com/pybamm-team/PyBaMM/blob/master/examples/notebooks/add-model.ipynb

[valentinsulzer]

First one should point here: https://pybamm.readthedocs.io/en/latest/source/models/lead_acid/higher_order.html
Second one should point here: https://github.com/pybamm-team/PyBaMM/tree/develop/examples/notebooks/Creating%20Models

[valentinsulzer]

Thanks! If all the broken links are fixed (can't see the test now) then I am happy to merge once the other tests pass

[priyanshuone6]

I have changed the timeout back to 10 seconds, I observed that making it 20 seconds adds more valid links in the "not pass" category.
Please check this for the output

[valentinsulzer]

@all-contributors add @priyanshuone6 for tests

[allcontributors]

@tinosulzer

I've put up a pull request to add @priyanshuone6! 🎉