Standard assert error message changes when test is run through nyc

[AndrewFinlay]

We recently bumped into this issue when running our internal test suite against node 10, and found differences in the result between nyc/mocha and just mocha. I'm assuming that this is caused by one of nyc dependencies and not nyc itself, however I really don't know so I figure this is the best place for the report.

Link to bug demonstration repository

demo

Expected Behavior

When running tests under node v10, the assertion failure message should be the same when running
npx mocha --recursive --ui bdd ./test and npx nyc mocha --recursive --ui bdd ./test

Observed Behavior

Running the test under mocha produces the node 10 error message, while running the test under nyc and mocha produces the node 8 error message.

Troubleshooting steps

• still occurring when I put cache: false in my nyc config
• minimal implementation in a fresh project
• run under different node versions, with or without nyc

Environment Information

  System:
    OS: macOS High Sierra 10.13.6
    CPU: (4) x64 Intel(R) Core(TM) i5-7600 CPU @ 3.50GHz
    Memory: 978.68 MB / 32.00 GB
  Binaries:
    Node: 8.16.0 - ~/.nvm/versions/node/v8.16.0/bin/node
    Yarn: 1.7.0 - /usr/local/bin/yarn
    npm: 6.4.1 - ~/.nvm/versions/node/v8.16.0/bin/npm
  npmPackages:
    nyc: ^14.1.1 => 14.1.1 

  Binaries:
    Node: 10.16.0 - ~/.nvm/versions/node/v10.16.0/bin/node
    Yarn: 1.7.0 - /usr/local/bin/yarn
    npm: 6.9.0 - ~/.nvm/versions/node/v10.16.0/bin/npm
  npmPackages:
    nyc: ^14.1.1 => 14.1.1 

[coreyfarrell]

Is it just showing node 8 error messages or is it actually running under node 8? Can the same happen if tests are run under node.js 12? If so we could test with class Foo { classSetField = 'value'; } which is only valid syntax in node.js 12.

Also I see that you are using npx in your npm test / npm run cover scripts. I've seen npx do strange things to the environment so I'm wondering if removing it helps.

I've got other things I have to look at this morning but I hope to investigate this afternoon.

[AndrewFinlay]

I think it's just showing the error message, process.version returns the expected node version number before each test regardless of nyc being involved. Just a quick note that I'm using nvm to switch between versions.

The issue still occurs under node 12. I tried adding the node 12 class field with a simple getter and a test, both mocha and nyc/mocha had no issue with node 12 class fields but the assert message still differed, node 10 complained in both cases as you'd expect.

Taking npx out of the picture and running with ./node_modules/.bin/nyc and ./node_modules/.bin/_mocha doesn't affect the outcome.

[coreyfarrell]

It appears that something is causing node.js to think it's in REPL when nyc is running. See https://nodejs.org/dist/latest/docs/api/assert.html#assert_assert_ok_value_message

@bcoe do you have any ideas on this? Switching from nyc built-in instrumentation to babel-plugin-istanbul instrumenter does not fix the issue.

[bcoe]

@coreyfarrell @AndrewFinlay interesting, there's a lot of fiddling with the environment in spawnWrap, perhaps it's triggering whatever check Node.js has in place for the REPL?

[coreyfarrell]

@bcoe Configuring nyc with "instrument": false prevents the issue from happening, so it's not spawn-wrap.

[AndrewFinlay]

@coreyfarrell, in an earlier test I couldn't reproduce the issue with stand alone instrumentation. I'll give it another try with the linked repo though.

[coreyfarrell]

I seem to be getting some inconsistent results or maybe I ran it incorrectly earlier, now the error is not happening if I pre-instrument lib/assert.js then run under nyc with instrumentation disabled.

[AndrewFinlay]

I've spent some time digging into this and found that this is a NodeJS issue and not a nyc issue.
I can recreate the problem using plain NodeJS with none of the gymnastics performed by nyc.

It seems that the assert exception message changes depending on the whitespace surrounding the code generating it. This means that uninstrumented code will generate one result, while the minified instrumented result will generate another. If you unminify the instrumented source it will go back to the original result.

I've put up an issue with a minimal reproduction on the NodeJS project here.
I've also updated the demo repository here to show the result of running nyc against uninstrumented, instrumented and unminified instrumented code.

As for current mitigations, if you include your own exception message assert(0, 'my message') it will always report as my message.

[coreyfarrell]

Thanks for investigating, I'm glad to hear you found the cause. I'd like to leave this report open until a fix is implemented in node.js.