-
-
Notifications
You must be signed in to change notification settings - Fork 35.1k
Description
I noticed this from nodejs/doc-kit#691 which was introduced by #57343 and this does not look right
make test-onlyis for functional tests. It's not for contributing or CI testing, but for embedders or distributors to verify the Node.js binary they build is functioning correctly. It should not run any code building the docs into a website or the having to support monolithic dependency as that's irrelevant for the goal.make test-onlyshould not require internet installs. Again it's only for people verifying the binary, you could just be scp-ing a tarball into a build server that does not necessarily have the permission to download from npm registry - and it'd be a pain having to work around it just so that you can test the Node.js binary works correctly, and do not care about the doc building at all.- As
make test-onlyis functional tests, it's what should be run on experimental platforms to verify the binary works correctly on a platform that's not yet well supported.doc-kitdepends on e.g. prebuilt addons/wasm that would make it unnecessarily difficult for no good reason, as can be seen from fix: disable wasm highlighter on riscv64 doc-kit#691
From a glance I think we should move https://github.com/nodejs/doc-kit/blob/main/src/generators/addon-verify/index.mjs back into core and replace it with a simple script with regexes that extract the artifacts. Unless there are other non-doc tests that depend on doc-kit - as far as I can tell the addon doc building is the only dependency, but it's an overkill having to e.g. run the minifier/highlighter (??!!) to extract the addons for testing. We can simply leave explicit markers in the markdown to deterministically know how the artifacts should be extracted, having to parse markdown and guess where they are based on headings is already hacky and brittle for this.