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

Set up dead link checker in CI #1330

Merged
merged 20 commits into from
Jul 23, 2023

Conversation

CarlosHSF99
Copy link
Contributor

Resolves #1328

Implementation was done following the example in the issue, but including json in the type of files to be tested to check the links.json files for the complementary links in the concepts pages.

Also fixed some links so as not to leave the repo in an invalid state for the test being added:

  • updated some urls to https
  • fixed slack link in resources
  • added the irc link in resources to the new .lycheeignore file as it is not supported by lychee

Tested with act, and it seems to be working, with every link passing the test.

@angelikatyborska
Copy link
Contributor

I would love to test it with act too, but I'm having some trouble using it. Can you tell me which docker image did you use? The medium one catthehacker/ubuntu:act-20.04 or the large one catthehacker/ubuntu:full-20.04?

@angelikatyborska
Copy link
Contributor

angelikatyborska commented Jun 22, 2023

Alright, I managed to get act to work with the medium docker container. I just needed the right flag because I'm on an M1 mac ( act -j linkChecker --container-architecture linux/amd64)

Something is not right. Lychee reports finding 46 links in total, but we have 60 concepts in this repo, each concept has two .md files and one .json file, I would say there's at least 10 links per concepts in those three files. So I would expect the total link count of the whole repo to be above 500, maybe even above 1000.

If I test and mess up a link on purpose in files for example concepts/basics/about.md, concepts/exceptions/links.json, or exercises/practice/affine-cipher/.meta/config.json, Lychee doesn't detect that 🤔

@angelikatyborska
Copy link
Contributor

That's more like it!

| | Status        | Count |
| |---------------|-------|
| | 🔍 Total      | 1158  |
| | ✅ Successful | 1108  |
| | ⏳ Timeouts   | 0     |
| | 🔀 Redirected | 0     |
| | 👻 Excluded   | 1     |
| | ❓ Unknown    | 0     |
| | 🚫 Errors     | 49    |

I'll fix the remaining errors in this PR and merge, thank you very much 💜

@CarlosHSF99
Copy link
Contributor Author

Thank you! Although I was still unsure of the results, I couldn't find the same number of links with a python script, and the lack of single quotes does not seem to cause a problem in the problem-specifications repo.

@github-actions
Copy link
Contributor

Thank you for contributing to exercism/elixir 💜 🎉. This is an automated PR comment 🤖 for the maintainers of this repository that helps with the PR review process. You can safely ignore it and wait for a maintainer to review your changes.

Based on the files changed in this PR, it would be good to pay attention to the following details when reviewing the PR:

  • General steps

    • 🏆 Does this PR need to receive a label with a reputation modifier (x:size/{tiny,small,medium,large,massive})? (A medium reputation amount is awarded by default, see docs)
  • Any exercise changed

    • 👤 Does the author of the PR need to be added as an author or contributor in <exercise>/.meta/config.json (see docs)?
    • 🔬 Do the analyzer and the analyzer comments exist for this exercise? Do they need to be changed?
    • 📜 Does the design file (<exercise>/.meta/design.md) need to be updated to document new implementation decisions?
  • Practice exercise changed

    • 🌲 Do prerequisites, practices, and difficulty in config.json need to be updated?
    • 🧑‍🏫 Are the changes in accordance with the community-wide problem specifiations?
  • Concept exercise changed

    • 🌲 Do prerequisites and practices in config.json need to be updated?
    • 📖 Does the concept introduction provide all necessary information to solve this exercise?
  • Concept changed

    • 👤 Does the author of the PR need to be added as an author or contributor in <concept>/.meta/config.json (see docs)?
    • 📖 Does the concept introduction provide all necessary information to solve this exercise?

Automated comment created by PR Commenter 🤖.

@angelikatyborska
Copy link
Contributor

angelikatyborska commented Jun 22, 2023

Related PR nr 1: exercism/problem-specifications#2293 (secret handshake)
Related PR nr 2: exercism/problem-specifications#2294 (users.csc.calpoly.edu links)

@CarlosHSF99
Copy link
Contributor Author

Fixed the ci.yml file. In the previous commit I forgot this one, having only fixed the pr.ci.yml.

@angelikatyborska
Copy link
Contributor

I did not expect how many links are going to pose problems for the link checker 🤦. Now it's twitter blocking not logged in users, and some random timeouts from other pages that aren't really broken links. I'll wait a bit and try to re run the check... I hope it won't always be so flaky.

@dabaer
Copy link
Contributor

dabaer commented Jul 3, 2023

@angelikatyborska Would it be helpful to enable the --cache flag? It seems Lychee doesn't cache results by default, which is slightly contributing to hitting rate limits on some domains.

@angelikatyborska
Copy link
Contributor

I lost my patience and ignored all the links that are giving false positives wrongly on CI (they work in the browser and they work with curl and with lychee from my local machine)

@angelikatyborska angelikatyborska merged commit d24ef90 into exercism:main Jul 23, 2023
11 checks passed
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.

Set up a dead link checker
3 participants