Skip to content

Commit

Permalink
fix: fix pathing issues and succeed/fail criteria with no files found
Browse files Browse the repository at this point in the history
  • Loading branch information
prototypicalpro committed Dec 4, 2020
1 parent 7df3086 commit c0c101b
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 31 deletions.
2 changes: 1 addition & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

41 changes: 19 additions & 22 deletions rules/file-no-broken-links.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,14 @@
// Copyright 2017 TODO Group. All rights reserved.
// SPDX-License-Identifier: Apache-2.0

const { HtmlChecker, reasons } = require('broken-link-checker')
const { HtmlChecker } = require('broken-link-checker')
const path = require('path')
const { URL } = require('url')
const GitHubMarkup = require('../lib/github_markup')
const Result = require('../lib/result')
// eslint-disable-next-line no-unused-vars
const FileSystem = require('../lib/file_system')
const { link } = require('fs')

// TODO: how to autoprefix domains with http or https?
/**
* Searches for a renderable markup document, renders it, and then
* checks for broken links by scanning the html.
Expand Down Expand Up @@ -51,17 +49,19 @@ async function fileNoBrokenLinks(fs, options) {
const htmlChecker = new HtmlChecker({
...options,
autoPrefix: [{ pattern: /^[\w_-]+\.[^\s]+$/i, prefix: 'https://' }], // autoprefix domain-only links
includeLink: (link) => !link.get('originalURL').startsWith('#') // exclude local section links
includeLink: link => !link.get('originalURL').startsWith('#') // exclude local section links
}).on('link', res => linkRes.push(Object.fromEntries(res.entries())))
await htmlChecker.scan(rendered, new URL(`file://${fs.targetDir}`))
await htmlChecker.scan(
rendered,
new URL(`file://${path.posix.join(fs.targetDir, '/')}`)
)
// find all relative links, and double check the filesystem for their existence
// filter down to broken links
console.log(JSON.stringify(linkRes))
const brokenLinks = linkRes.filter(({ isBroken }) => isBroken)
// split into invalid and otherwise failing
const { failing, invalid } = brokenLinks.reduce(
(res, linkRes) => {
linkRes.brokenReason === reasons.BLC_INVALID
linkRes.brokenReason === 'BLC_INVALID'
? res.invalid.push(linkRes)
: res.failing.push(linkRes)
return res
Expand All @@ -70,38 +70,35 @@ async function fileNoBrokenLinks(fs, options) {
)
// make the messages for the failing URLs
const failingMessages = failing.map(
({
brokenReason,
originalURL,
http,
}) =>
({ brokenReason, originalURL, http }) =>
`${originalURL} (${
brokenReason.includes('HTTP')
? `status code ${http?.response?.statusCode}`
? `status code ${
http && http.response && http.response.statusCode
}`
: `unknown error ${brokenReason}`
})`
)
// process the invalid links to check if they're actually filesystem paths
// returning the message for invalid URLs
const failingInvalidMessagesWithNulls = await Promise.all(
invalid.map(async b => {
const {
resolvedURL,
originalURL
} = b
let url;
const { resolvedURL, originalURL } = b
let url
// parse the URL, and if it fails to parse it's invalid
try {
url = new URL(resolvedURL);
url = new URL(resolvedURL)
if (url.protocol !== 'file:' || !url.pathname)
return `${resolvedURL} (invalid URL)`;
} catch { return `${originalURL} (invalid path)`; }
return `${resolvedURL} (invalid URL)`
} catch (e) {
return `${originalURL} (invalid path)`
}
// verify the path is relative, else the path is invalid
if (path.posix.isAbsolute(originalURL))
return `${originalURL} (invalid path)`
// verify the path doesn't traverse outside the project, else the path is excluded
const targetDir = path.posix.resolve(fs.targetDir)
const filePath = path.posix.join('/', url.host, url.pathname);
const filePath = path.posix.join('/', url.host, url.pathname)
const absPath = path.posix.resolve(targetDir, filePath)
const relPath = path.posix.relative(targetDir, absPath)
if (relPath.startsWith('..')) return null
Expand Down
21 changes: 14 additions & 7 deletions tests/rules/file_no_broken_links_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,9 @@ describe('rule', () => {
const scope = nock('https://www.example.com')
.head('/something/somethingelse')
.reply(200)
const scope2 = nock('www.example.com').head('/something').reply(200)
const scope2 = nock('https://www.example.com')
.head('/something')
.reply(200)

const ruleopts = {
globsAll: ['multiple_links.md']
Expand All @@ -271,9 +273,7 @@ describe('rule', () => {
const scope = nock('https://www.example.com')
.head('/something/somethingelse')
.reply(200)
const scope2 = nock('https://www.example.com')
.head('/something')
.reply(200)
.persist()

const ruleopts = {
globsAll: ['link.md', 'link.rst']
Expand All @@ -293,7 +293,6 @@ describe('rule', () => {
})

scope.done()
scope2.done()
})

it('fails if no files are found', async () => {
Expand All @@ -304,7 +303,11 @@ describe('rule', () => {
const actual = await fileNoBrokenLinks(testFs, ruleopts)

expect(actual.passed).to.equal(false)
expect(actual.targets).to.have.length(0)
expect(actual.targets).to.have.length(1)
expect(actual.targets[0]).to.deep.include({
passed: false,
pattern: 'notafile'
})
})

it('succeeds if no files are found and succeed-on-non-existent is true', async () => {
Expand All @@ -316,7 +319,11 @@ describe('rule', () => {
const actual = await fileNoBrokenLinks(testFs, ruleopts)

expect(actual.passed).to.equal(true)
expect(actual.targets).to.have.length(0)
expect(actual.targets).to.have.length(1)
expect(actual.targets[0]).to.deep.include({
passed: false,
pattern: 'notafile'
})
})
}
})
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
[my file](./apath/toafile#thing)
[my file](./link.md#section)

0 comments on commit c0c101b

Please sign in to comment.