Skip to content
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

test-make-doc now failing locally #19694

Closed
mcollina opened this issue Mar 30, 2018 · 12 comments
Closed

test-make-doc now failing locally #19694

mcollina opened this issue Mar 30, 2018 · 12 comments
Labels
build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. test Issues and PRs related to the tests. tools Issues and PRs related to the tools directory.

Comments

@mcollina
Copy link
Member

  • Version: master
  • Platform: mac
  • Subsystem: doc, test

I just pulled and test-make-doc is now failing:

$ out/Release/node /Users/matteo/Repositories/node/test/doctool/test-make-doc.js
assert.js:248
    throw err;
    ^

AssertionError [ERR_ASSERTION]: all.html is empty
    at Object.<anonymous> (/Users/matteo/Repositories/node/test/doctool/test-make-doc.js:47:10)
    at Module._compile (internal/modules/cjs/loader.js:678:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:689:10)
    at Module.load (internal/modules/cjs/loader.js:589:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:528:12)
    at Function.Module._load (internal/modules/cjs/loader.js:520:3)
    at Function.Module.runMain (internal/modules/cjs/loader.js:719:10)
    at startup (internal/bootstrap/node.js:225:19)
    at bootstrapNodeJSCore (internal/bootstrap/node.js:572:3)

This as been introduced in #19581. Not sure what I should do to make this pass again on my machine, I already tried make clean and make doc.

@mcollina mcollina changed the title test-make-dock now failing locally test-make-doc now failing locally Mar 30, 2018
@mcollina
Copy link
Member Author

cc @vsemozhetbyt

@vsemozhetbyt
Copy link
Contributor

vsemozhetbyt commented Mar 30, 2018

Throwing assertion.

Cause of the added check:

Makefile generates docs by getting the console.log() output from the tools/doc/generate.js and redirecting it to the doc files. If tools/doc/generate.js throws, an empty doc file is still created. So we can have a situation when the last .md source has an error and all the needed files are nevertheless present, just one of them is empty. So we need to check if all the files are not empty.

  1. Can you check if there is an empty all.html file in out/doc/api folder?
  2. Can you check if building of all.html is not broken during the process?

@vsemozhetbyt
Copy link
Contributor

Can there also be some race condition? Can doctool/test-make-doc.js run till doc building is finished?

@mcollina
Copy link
Member Author

Can you check if there is an empty all.html file in out/doc/api folder?

Yes it is empty,.

Can you check if building of all.html is not broken during the process?

how?

Can there also be some race condition?

This fails even if I run this individually, after make doc has finished. There shouldn't be race conditions.

Can doctool/test-make-doc.js run till doc building is finished?

I do not understand. can you rephrase?

@vsemozhetbyt
Copy link
Contributor

how?

Maybe run make doc-only after deleted out/doc/api folder and check the output on console? There should be lines concerning all the doc files. (~102)

Can doctool/test-make-doc.js run till doc building is finished?

It seems this is not possible if you "run this individually, after make doc has finished." I thought there may be parallel processes of doc building and tests,

@mcollina
Copy link
Member Author

Maybe run make doc-only after deleted out/doc/api folder and check the output on console? There should be lines concerning all the doc files. (~102)

That did the trick. I think we should have make clean remove this folder as well.

@vsemozhetbyt vsemozhetbyt added doc Issues and PRs related to the documentations. build Issues and PRs related to build files or the CI. test Issues and PRs related to the tests. tools Issues and PRs related to the tools directory. labels Mar 30, 2018
@lpinca
Copy link
Member

lpinca commented Mar 31, 2018

make distclean already deletes the whole out folder.

@mcollina
Copy link
Member Author

Following the principle of least surprise, a developer expects make clean to restore the tree in a state that would have “make test” pass.

@vsemozhetbyt
Copy link
Contributor

Is there a way to make make rebuild a doc if it is empty?

@joyeecheung
Copy link
Member

joyeecheung commented Mar 31, 2018

Following the principle of least surprise, a developer expects make clean to restore the tree in a state that would have “make test” pass.

+1. Anyone knows why make clean does not just wipe the out folder? cc @nodejs/build

@MylesBorins
Copy link
Contributor

Just got hit by this on 10.x-staging

@jasnell
Copy link
Member

jasnell commented Oct 19, 2018

I think this is resolved now?

@jasnell jasnell closed this as completed Oct 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. test Issues and PRs related to the tests. tools Issues and PRs related to the tools directory.
Projects
None yet
Development

No branches or pull requests

6 participants