test: tests for module not to use sync functions

[shisama]

this refactors tests for module to reduce runtime

Refs: #20128

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• commit message follows commit guidelines

[addaleax]

@nodejs/testing

[Trott]

This doesn't fix the reference PR. Please use "Refs:" rather than "Fixes:" in the commit message. Otherwise, when this lands, it will automatically close that issue.

[Trott]

Unfortunately, these changes make at least one of the tests unreliable:

$ tools/test.py -j96 --repeat 192 test/parallel/test-module-loading-globalpaths.js
=== release test-module-loading-globalpaths ===                    
Path: parallel/test-module-loading-globalpaths
(node:77494) ExperimentalWarning: The fs.promises API is experimental
assert.js:75
  throw new AssertionError(obj);
  ^

AssertionError [ERR_ASSERTION]: Input A expected to strictly equal input B:
+ expected - actual

- ''
+ '$HOME/.node_modules'
    at child_process.execFile (/Users/trott/io.js/test/parallel/test-module-loading-globalpaths.js:61:18)
    at exithandler (child_process.js:289:5)
    at ChildProcess.errorhandler (child_process.js:301:5)
    at ChildProcess.emit (events.js:182:13)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:233:12)
    at onErrorNT (internal/child_process.js:404:16)
    at process._tickCallback (internal/process/next_tick.js:63:19)
Command: out/Release/node /Users/trott/io.js/test/parallel/test-module-loading-globalpaths.js

[shisama]

@Trott
Thank you for your comment.
I fixed commit comment.
The error doesn't occur on my macbook.

[Trott]

The error doesn't occur on my macbook.

@shisama Did you run it with the CLI flags like this?:

tools/test.py -j96 --repeat 192 test/parallel/test-module-loading-globalpaths.js

On the MacBook I'm using right now, that command takes 1 minute and 18 seconds to complete, most of the time during which the macbook is painfully slow. I got 163 successful runs and 22 failures. (That's strange because those numbers should add up to 192, but they don't.)

If they are all passing for you, then you likely have a much more powerful MacBook than I do. :-D But we shouldn't optimize our tests for such things.

[shisama]

@Trott

Did you run it with the CLI flags like this?:

Yes. I had run the command several times. But the error didn't occur.

But we shouldn't optimize our tests for such things.

OK. Do I need to close this pull request?

[Trott]

Yes. I had run the command several times. But the error didn't occur.

That's very surprising to me and I'm interested to find out why.

• You definitely have this branch checked out when you run the test? You're not running it on the master branch by accident or anything like that?
• You're definitely using tools/test.py -j96 --repeat 192 test/parallel/test-module-loading-globalpaths.js to run the test?
• Can you copy/paste the command and the result?

[shisama]

Can you copy/paste the command and the result?

I tried the command again, the error didn't occur.
The below are the command, the result, version, and branch name.

$ python tools/test.py -j96 --repeat 192 test/parallel/test-module-loading-globalpaths.js
[01:36|% 100|+ 192|-   0]: Done
$ ./node -v
v11.0.0-pre
$ git branch --contains=HEAD
* improve-module-tests

[Trott]

@shisama I don't have a good explanation for this (at least not yet), but I find that when I run the command like you do (python tools/test.py -j96 --repeat 192 test/parallel/test-module-loading-globalpaths.js), then it does not fail, but if I run it the way I described (exactly the same as you do except don't put python at the beginning, so tools/test.py -j96 --repeat 192 test/parallel/test-module-loading-globalpaths.js), then I get failures. Do you get the same result?

[shisama]

@Trott I tried the command and it didn't fail.
The result is:

tools/test.py -j96 --repeat 192 test/parallel/test-module-loading-globalpaths.js
[01:09|% 100|+ 192|-   0]: Done

git branch --contains=HEAD
* improve-module-tests

[Trott]

@shisama My guess, but it's only a guess, is that you have more memory and/or CPU than I do. Let's look at CI instead of local computers.

I tried running the command on a variety of machines on the Node.js CI to make sure it's not just my setup. Here's a small bit of output on AIX (viewable at https://ci.nodejs.org/job/node-stress-single-test/1872/nodes=aix61-ppc64/consoleText):

not ok 3 parallel/test-module-loading-globalpaths
  ---
  duration_ms: 104.599
  severity: fail
  exitcode: 1
  stack: |-
    (node:10485788) ExperimentalWarning: The fs.promises API is experimental
    assert.js:75
      throw new AssertionError(obj);
      ^
    
    AssertionError [ERR_ASSERTION]: Input A expected to strictly equal input B:
    + expected - actual
    
    - ''
    + '$NODE_PATH'
        at child_process.execFile (/home/iojs/build/workspace/node-stress-single-test/nodes/aix61-ppc64/test/parallel/test-module-loading-globalpaths.js:61:18)
        at exithandler (child_process.js:289:5)
        at ChildProcess.errorhandler (child_process.js:301:5)
        at ChildProcess.emit (events.js:182:13)
        at Process.ChildProcess._handle.onexit (internal/child_process.js:233:12)
        at onErrorNT (internal/child_process.js:404:16)
        at process._tickCallback (internal/process/next_tick.js:63:19)
  ...

Other platforms ran without a problem, so this is by no means universal.

Nonetheless, I think this is a genuine problem, especially if we are going to be doing this to other tests. We're going to start seeing things like this, and timeouts, and EAGAIN errors on calls to spawn().

[Trott]

(Although I should make sure that this isn't already happening on the master branch with this test as it currently stands. I'll run a stress test for that too and we'll see.)

[apapirovski]

@shisama Thanks for taking this on but I don't think many collaborators (including myself) think this is a move in the right direction. I'm going to close this out but I hope it doesn't discourage you from contributing in the future.

[shisama]

@apapirovski OK, never mind! I will continue to contribute in the future. Thanks for your comment.