test: continue normalizing fixtures use

[maasencioh]

Continues with the job proposed by @jasnell (there's still 217 occurrences in test/parallel)

Refs: #14332

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• commit message follows commit guidelines

Affected core subsystem(s)

test

[maasencioh]

I had to add this because the common package is not used after this change and therefore it conflicted withno-unused-vars

[tniessen]

You can just use require('../common').

[jasnell]

LGTM with green CI

[Trott]

Pretty sure an encoding needs to be sent to readKey() if you want a string rather than a buffer. Probably best to keep the tests behavior the same, so we probably want to do that...

[targos]

@Trott this is also true for fs.readFileSync(). No encoding was passed before so the behavior should be the same.

[Trott]

@Trott this is also true for fs.readFileSync(). No encoding was passed before so the behavior should be the same.

@targos You're right, of course. Never mind me.

Comments were wrong. Dismissing.

[maasencioh]

I solved the conflicts with master and after that I have this error:

=== release test-require-symlink ===                                           
Path: parallel/test-require-symlink
assert.js:42
  throw new errors.AssertionError({
  ^

AssertionError [ERR_ASSERTION]: 1 === 0
    at ChildProcess.<anonymous> (/home/miguel/github/node/test/parallel/test-require-symlink.js:58:12)
    at emitTwo (events.js:125:13)
    at ChildProcess.emit (events.js:213:7)
    at maybeClose (internal/child_process.js:935:16)
    at Socket.stream.socket.on (internal/child_process.js:355:11)
    at emitOne (events.js:115:13)
    at Socket.emit (events.js:210:7)
    at Pipe._handle.close [as _onclose] (net.js:547:12)
Command: out/Release/node --preserve-symlinks /home/miguel/github/node/test/parallel/test-require-symlink.js
[02:02|% 100|+ 1758|-   1]: Done                                               
make: *** [Makefile:199: test] Error 1

Any idea why if I don't modify this file I got an error there?

[jasnell]

It's because you modified this file: test/fixtures/module-require-symlink/symlinked.js

Generally, I would not change any of the files in the actual fixtures directory.

[maasencioh]

indeed @jasnell , I had a typo in there, now all the test are passing, but I don't know if you want me to revert the ones in fixtures then?

[jasnell]

So long as all of the tests are passing, then it should be fine with the fixtures files edits. Those just tend to be very touchy with a number of the tests so it's best to be cautious. We'll get a good CI run and see where we're at.

I really appreciate you doing this btw!

[maasencioh]

it's a great pleasure, I didn't have time to touch all the ones in parallel, but it's on my bucket list!

[tniessen]

I know we are not too strict about this in other test files, but if you get to test/parallel, remember to keep the imports in ASCII order.

CI: https://ci.nodejs.org/job/node-test-pull-request/9852/

[BridgeAR]

Landed in 9a5c3cf

[maasencioh]

sorry to @tniessen , I didn't knew it but I'll have it in mind for next pr, thank you!