url checker report in Github

[shahzebsiddiqui]

Just a thought, the URLchecker is a nice feature to be honest it is not practical to go into the output of the Action to see the error report. It would be nice to have a bot report all the broken links as part of the check in the PR. The code reviewers and authors would be keen on getting this information in their PR so they can fix their code.

In this case, the bot just reports only failure, if there are none have a custom message to your liking saying SUCCESS: all URL links are valid!!

Once this is implemented, i am sure it would be very useful for lots of folks.

[SuperKogito]

This is a great suggestion, I totally agree. I have this along other useful features in mind. I will be looking into this first 😉

[shahzebsiddiqui]

glad i can help. Let me know once you have a new version and I can test it out. I am already using the URLchecker as part of buildtest https://github.com/HPC-buildtest/buildtest-framework and found it to be useful in fixing some bugs.

I think the bot notification in PR would be useful when adding documentation where you will find reference to links that need to be tested.

[SuperKogito]

This is available in the latest release 0.1.4, please refer to the README for the documentation and this URLs-checker-test-repo if you need an example. feel free, to ask and report on this feature here.

[shahzebsiddiqui]

I have updated the version see buildtesters/buildtest#202 do you have Release notes on the changes between the version. I can see that now its reporting only failures, before it was reporting all URLs including passed and failures. It certainly helps narrow down the result. I also see that the URLcheck is now failing due to the error before it was passing even if there were some broken links. I guess its a good step towards this. I was looking for a bot notification that comments in GitHub for broken links.

[shahzebsiddiqui]

@SuperKogito also i see many urls that are valid that are falsely reported as a broken link. I think there is something that needs to be looked at.

Please see https://github.com/HPC-buildtest/buildtest-framework/pull/202/checks?check_run_id=492841895

[shahzebsiddiqui]

I tried to add a white list for some of the URLs which i know are valid but it still reported them as failure. See buildtesters/buildtest#202. I used whited_listed_urls.

[SuperKogito]

I can see that the transition to v0.1.4 was not as smooth as I expected. I noted all your feedback and hopefully I will be looking into fixing these issues in the upcoming days.

[shahzebsiddiqui]

@SuperKogito it would help to note if the URL is breaking due to trailing / at end of link or not.

For instance i see the following

 buildtest-framework/docs/installing_buildtest.rst 
 -------------------------------------------------
                               https://lmod.readthedocs.io/en/latest/030_installing.html .

The link is valid if you navigate to https://lmod.readthedocs.io/en/latest/030_installing.html i see many other links that are false positive. Even link to marketplace failed such as the URLchecker

 buildtest-framework/CHANGELOG.rst 
 ---------------------------------
                                                          https://github.com/marketplace/actions/urls-checker .

[vsoch]

@SuperKogito and @shahzebsiddiqui we originally ran into the issue of having comma and space separated links in the whitelist, and I suggested testing without the space only because it would make sense for a bash envar to not have spaces. One follow up to that, I noticed while working on #17 that the whitelisted urls example does show spaces, so as part of my PR I fixed that up.

@SuperKogito if there is something else I can address here and you don't have the bandwidth, please let me know! I can open another PR after #17.

[SuperKogito]

@ vsoch I think your use of url.strip() solved these issues in #17, the issues mentioned here seem to be solved. @shahzebsiddiqui @vsoch if you can confirm that, please close this.

[vsoch]

Yes this is correct! This was fixed and now merged into master, thanks @SuperKogito!