-
-
Notifications
You must be signed in to change notification settings - Fork 32.2k
test: test make doc and verify toc #16208
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
Closed
Closed
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
d8cc63b
test: add common.projectDir
joyeecheung 43ce6da
test: test make doc and verify toc
joyeecheung 48e3b30
doc projectDir
joyeecheung 882172f
put the make doc part in Makefile
joyeecheung 2ff4a77
fix lint
joyeecheung 271ff20
s/doc-only/doc/
joyeecheung 60c3c85
put make doc in build-ci
joyeecheung 014e360
remove redundant slash
joyeecheung f678717
put doc in the dependency of test-ci
joyeecheung bdff9e3
fix nits
joyeecheung 2de566b
fix nits: common
joyeecheung File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
'use strict'; | ||
const common = require('../common'); | ||
if (common.isWindows) { | ||
common.skip('`make doc` does not run on Windows'); | ||
} | ||
|
||
// This tests that `make doc` generates the documentation properly. | ||
// Note that for this test to pass, `make doc` must be run first. | ||
|
||
const assert = require('assert'); | ||
const fs = require('fs'); | ||
const path = require('path'); | ||
|
||
const apiPath = path.resolve(common.projectDir, 'out', 'doc', 'api'); | ||
const docs = fs.readdirSync(apiPath); | ||
assert.ok(docs.includes('_toc.html')); | ||
|
||
const toc = fs.readFileSync(path.resolve(apiPath, '_toc.html'), 'utf8'); | ||
const re = /href="([^/]+\.html)"/; | ||
const globalRe = new RegExp(re, 'g'); | ||
const links = toc.match(globalRe); | ||
assert.notStrictEqual(links, null); | ||
|
||
// Test that all the relative links in the TOC of the documentation | ||
// work and all the generated documents are linked in TOC. | ||
const linkedHtmls = links.map((link) => link.match(re)[1]); | ||
for (const html of linkedHtmls) { | ||
assert.ok(docs.includes(html), `${html} does not exist`); | ||
} | ||
|
||
const excludes = ['.json', '_toc', 'assets']; | ||
const generatedHtmls = docs.filter(function(doc) { | ||
for (const exclude of excludes) { | ||
if (doc.includes(exclude)) { | ||
return false; | ||
} | ||
} | ||
return true; | ||
}); | ||
|
||
for (const html of generatedHtmls) { | ||
assert.ok(linkedHtmls.includes(html), `${html} is not linked in toc`); | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this won't work on our cross-compiling Raspberry Pi devices in CI. @nodejs/build ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmmm...but it did pass in the CI, so...hmmm....
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, perhaps CI was run before that specific feature was added? I'm certainly seeing it fail 100% of the time in CI right now as in https://ci.nodejs.org/job/node-test-binary-arm/11032/.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if the problem isn't common.projectDir at all but instead that the Pi devices run
make test-ci-js
and notmake test
ormake test-ci
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(...and so it skips the
make doc
....which we probably want it to skip because who knows how long it would take to run on a Pi and we'd be running it on all of them just for a test that only one of them runs...)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I've got a fix in #16301 but we'll see if CI agrees....
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I wonder why this passed the CI before...anyway thanks for the fix!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@joyeecheung I think it passed CI because this change wasn't in the branch when CI ran. It was run before a5262f1 landed.