Investigate flaky test-fs-readfile-tostring-fail

[Trott]

• Version: v9.0.0-pre on CI
• Platform: osx 1010
• Subsystem: test

https://ci.nodejs.org/job/node-test-commit-osx/13607/nodes=osx1010/console

not ok 1990 sequential/test-fs-readfile-tostring-fail
  ---
  duration_ms: 0.506
  severity: fail
  stack: |-
    /Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1010/test/sequential/test-fs-readfile-tostring-fail.js:60
      throw err;
      ^
    
    AssertionError [ERR_ASSERTION]: false == true
        at /Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1010/test/sequential/test-fs-readfile-tostring-fail.js:34:12
        at /Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1010/test/common/index.js:533:15
        at FSReqWrap.readFileAfterClose [as oncomplete] (fs.js:528:3)

[Leko]

node/test/sequential/test-fs-readfile-tostring-fail.js

Line 34 in e1c29f2

assert.ok(err instanceof Error);

I think better is assert.equal(err.constructor, Error) to display more information.
If err is not an instance of Error (ex. Number), it will display constructor name.
It would be nicer than assert(err instanceof Error).

> const err = new Error()
undefined

> assert(err instanceof Number)
AssertionError [ERR_ASSERTION]: false == true

> assert.equal(err.constructor, Number)
AssertionError [ERR_ASSERTION]: { [Function: Error] stackTraceLimit: 10, prepareStackTrace: undefined } == [Function: Number]

My opinion does not solve the issue but I think that it will provide useful information at next time the same problem occurs.
How do you think?

[joyeecheung]

@Leko If the error is not an Error I think in this case it's basically a null. The question is why the read/toString() succeeded here.

[Leko]

it's basically a null

@joyeecheung Ah, I see. It’s just nothing.

[apapirovski]

This seems to be failing reasonably often again. Anyone have any ideas?

https://ci.nodejs.org/job/node-test-commit-osx/16147/nodes=osx1010/tapResults/

[MylesBorins]

another failure

https://ci.nodejs.org/job/node-test-commit-osx/16187/nodes=osx1010/console

[BridgeAR]

This does not only fail on OS-X as it seems:

https://ci.nodejs.org/job/node-test-commit-linux/16441/nodes=ubuntu1404-64/console

[gireeshpunathil]

Easily reproduced with adjusting ulimits:

#ulimit -f 10000000
#./node test/sequential/test-fs-readfile-tostring-fail.js
#ulimit -f 1000000
#./node test/sequential/test-fs-readfile-tostring-fail.js

/home/gireesh/node/test/sequential/test-fs-readfile-tostring-fail.js:67
  throw err;
  ^

AssertionError [ERR_ASSERTION]: The expression evaluated to a falsy value:

  assert.ok(err instanceof Error)

    at /home/gireesh/node/test/sequential/test-fs-readfile-tostring-fail.js:34:12
    at /home/gireesh/node/test/common/index.js:474:15
    at FSReqWrap.readFileAfterClose [as oncomplete] (fs.js:424:3)

with this patch it shows the error was null - evidently the write failed, so the read succeeded.

--- a/test/sequential/test-fs-readfile-tostring-fail.js
+++ b/test/sequential/test-fs-readfile-tostring-fail.js
@@ -31,6 +31,7 @@ for (let i = 0; i < 201; i++) {
 stream.end();
 stream.on('finish', common.mustCall(function() {
   fs.readFile(file, 'utf8', common.mustCall(function(err, buf) {
+    console.log(err)
     assert.ok(err instanceof Error);

#./node test/sequential/test-fs-readfile-tostring-fail.js

null
...

#l /home/gireesh/node/test/.tmp/toobig.txt
-rw-r--r--  1 gireeshpunathil  staff  1024000000 May 16 22:06 /home/gireesh/node/test/.tmp/toobig.txt

So I am not claiming that th CI had ulimit -f set to low values, but under differing fs situations, such a circumstances would have become in effect.

I guess the test should validate that kStringMaxLength bytes of data is indeed written, before making such an assertion.

[BridgeAR]

And again: https://ci.nodejs.org/job/node-test-commit-osx/18661/nodes=osx1010/console