Skip to content

Conversation

@peixotoleonardo
Copy link
Contributor

When only documentation is changed, running tests may not be necessary. Perhaps we could consider skipping them in such cases.

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/actions

@nodejs-github-bot nodejs-github-bot added the meta Issues and PRs related to the general management of the project. label Nov 22, 2024
Copy link
Member

@richardlau richardlau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have tests that check the docs for consistency, e.g.

const cliMd = path.join(rootDir, 'doc', 'api', 'cli.md');
const cliText = fs.readFileSync(cliMd, { encoding: 'utf8' });

@peixotoleonardo
Copy link
Contributor Author

@richardlau

This test ensures that all CLI options are documented but does not validate other parts of the documentation.

I put !doc/api/cli.md to run the test if this file change.

@richardlau
Copy link
Member

@richardlau

This test ensures that all CLI options are documented but does not validate other parts of the documentation.

I put !doc/api/cli.md to run the test if this file change.

There are others, e.g.

const changelogPath = `doc/changelogs/CHANGELOG_V${major}.md`;
(I don't have time to find them all right now -- the point is that we do have tests that are looking at the docs).

@richardlau
Copy link
Member

The addons tests also extract the code snippets from the documentation to test those:

node/Makefile

Lines 369 to 387 in 3178a76

DOCBUILDSTAMP_PREREQS = tools/doc/addon-verify.mjs doc/api/addons.md
ifeq ($(OSTYPE),aix)
DOCBUILDSTAMP_PREREQS := $(DOCBUILDSTAMP_PREREQS) out/$(BUILDTYPE)/node.exp
endif
ifeq ($(OSTYPE),os400)
DOCBUILDSTAMP_PREREQS := $(DOCBUILDSTAMP_PREREQS) out/$(BUILDTYPE)/node.exp
endif
node_use_openssl_and_icu = $(call available-node,"-p" \
"process.versions.openssl != undefined && process.versions.icu != undefined")
test/addons/.docbuildstamp: $(DOCBUILDSTAMP_PREREQS) tools/doc/node_modules
@if [ "$(shell $(node_use_openssl_and_icu))" != "true" ]; then \
echo "Skipping .docbuildstamp (no crypto and/or no ICU)"; \
else \
$(RM) -r test/addons/??_*/; \
[ -x $(NODE) ] && $(NODE) $< || node $< ; \
[ $$? -eq 0 ] && touch $@; \
fi

@peixotoleonardo
Copy link
Contributor Author

@richardlau

This case I will close the PR, sorry about my English (It's in progress).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

meta Issues and PRs related to the general management of the project.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants