Adding test task to gulpfile that will check all links #103
Conversation
etsauer
changed the title
etsauer
changed the title
etsauer
force-pushed
the
npm-test-suite
branch
from
b0a1374 to
de2af29
Compare
There was a problem hiding this comment.
This looks awesome! The link checker works well, and it looks like a lot of broken links were fixed.
I added some comments on the code itself. FYI, it looks like the link checker considers 403 redirects to be broken links, even though they redirect to a valid page. For example, the Jenkins wiki appears to have moved, so our links to that wiki show up as errors.
It looks like there is at least 1 broken internal link remaining:
├─BROKEN─ http://localhost:26388/articles/app_dev/APM_guidance.html (HTTP_404)
The build fails if broken links are found, right? It's worth noting that if a site that we link to goes down, our build will fail until either the site is restored or we remove the link. I think that came up for us a while back.
What do you think about setting the build to continue even if this check fails, at least for now? That way, we could merge this PR and fix the remaining issues in a future PR.
test/link-checker.js
Outdated
| }, | ||
| junk: function(result) | ||
| { | ||
| // if (logOptions.excludeFilteredLinks === true) |
gulpfile.babel.js
Outdated
| process.on('uncaughtException', function (err) { | ||
| console.log(err); | ||
| }); | ||
| var min = Math.ceil(10000); |
There was a problem hiding this comment.
Why are these values rounded? Is there some weird edge case where you were getting a decimal point?
| link: function(result) | ||
| { | ||
| // Exclude cached links only if not broken | ||
| if (result.broken===false && result.http.cached===true) |
There was a problem hiding this comment.
Should links like this also be excluded?
├─BROKEN─ http://router.default.svc.cluster.local:1936/healthz (ERRNO_ENOTFOUND)├─BROKEN─ https://console.c1-ocp.myorg.com:8443/healthz (ERRNO_ENOTFOUND)
|
@rdebeasi comments addressed. |
|
In chat, we talked about possibly making build status and logs visible to the public. It sounds like that work would come later, though. For now, broken links will cause the builds to fail, and developers can see the link check result by running the build locally or asking someone with access to Jenkins. These changes look good to me. Are you ready for them to be merged? |
|
@rdebeasi book it. we'll clean up quickly as needed. |
What is this PR About?
Adding a test suite that we can use to check links and other quality validations.
How should we test or review this PR?
Then, in another window...
TODOs:
servertask into thetestso that we don't need two windowsIs there a relevant Trello card or Github issue open for this?
#18
Who would you like to review this?
cc: @redhat-cop/cant-contain-this
Please note: we may close your PR without comment if you do not check the boxes above and provide ALL requested information.