test: n-api/addons test change issues

[mscdex]

• Version: master, v8.x
• Platform: n/a
• Subsystem: n-api, test

Is there supposed to be a way to correctly "fix" n-api/addon tests when those tests change? Today was the second time in the past few days I've ran into issues like this:

Building addon /home/mscdex/git/node/test/addons-napi/test_napi_status/
gyp: binding.gyp not found (cwd: /home/mscdex/git/node/test/addons-napi/test_napi_status) while trying to load binding.gyp
Makefile:288: recipe for target 'test/addons-napi/.buildstamp' failed
make[1]: *** [test/addons-napi/.buildstamp] Error 1
Makefile:196: recipe for target 'test' failed
make: *** [test] Error 2

or this (which I originally got earlier today):

=== release testNapiStatus ===                                                 
Path: addons-napi/test_general/testNapiStatus
/home/mscdex/git/node/test/addons-napi/test_general/testNapiStatus.js:7
addon.createNapiError();
      ^

TypeError: addon.createNapiError is not a function
    at Object.<anonymous> (/home/mscdex/git/node/test/addons-napi/test_general/testNapiStatus.js:7:7)
    at Module._compile (module.js:569:30)
    at Object.Module._extensions..js (module.js:580:10)
    at Module.load (module.js:503:32)
    at tryModuleLoad (module.js:466:12)
    at Function.Module._load (module.js:458:3)
    at Function.Module.runMain (module.js:605:10)
    at startup (bootstrap_node.js:158:16)
    at bootstrap_node.js:575:3

Performing a make test-addons-clean did not fix the problem. Is there something else we should be doing (in general)?

/cc @nodejs/n-api @nodejs/build

[addaleax]

There’s an extra target make test-addons-napi-clean, does that help?

[mscdex]

@addaleax The test-addons-clean target already calls that target as its last step.

[mhdawson]

The clean must not remove the dynamically built contents from the addon subdirectories directories. I wonder if this is specific to n-api or just that they are changing more often than the non n-api ones.

[mhdawson]

Looking at the issue it may be more that after a pull, that pull is not deleting directories that were removed and therefore the test still tries to run.

[mscdex]

@mhdawson That was my guess, leftover directories.

[joyeecheung]

@mhdawson test-addons-clean cleans the dynamically generated directories with $(RM) -r test/addons/??_*/ but there doesn't seem to be a naming pattern for the generated directories of n-api. They can be written out in the Makefile one by one, although that takes a bit more maintenance.

[joyeecheung]

Ah I understand the comments incorrectly, this is about git pull not deleting the directories...but then those leftover directories should show up in git status output right?

[mscdex]

@joyeecheung Not from what I saw earlier.

[mhdawson]

The errors in the original posts are both related to directories that I had deleted when I consolidated them into other tests to cut down on the number of addons that we have to build each time.

[mhdawson]

I had to manually delete those directories after doing a git pull. What we would need is a way to tell if its a directories that was deleted as opposed to a new one added. Not sure if we can safely do that.

[gibfahn]

but there doesn't seem to be a naming pattern for the generated directories of n-api.

Seems to me that the ideal fix would be to make the generated directories follow a pattern. Then you could use make test-addons-clean. Not sure how easy that would be though.

[mhdawson]

The directories are not generated, tests are committed to sub-directories and the test automatically scans/runs tests in those directories

[Trott]

Do we have a proposed solution for this? I'm guessing we don't want to run git commands from the Makefile, but if that's not a code smell, then we could do that I suppose... git clean -fX test/addons-napi or something like that. (Haven't tested it myself.)