Flaky test in nyc's test suite #1259

AndrewFinlay · 2019-12-30T02:34:51Z

Link to bug demonstration repository

This one ;)

I'm finding one of the tests is flaky in the nyc test suite. The test is 'nyc displays help to stderr when it doesn\'t know what to do' in test/nyc-integration.js. It fails on the check t.equal(stderr, help.stdout).

Expected Behavior

Running npm t on nyc's current master repo produces consistent results

Observed Behavior

Running npm t on nyc's current master repo produces inconsistent results due to a flaky test,
'nyc displays help to stderr when it doesn\'t know what to do', in test/nyc-integration.js.

Troubleshooting steps

Environment Information

npx: installed 1 in 0.814s

  System:
    OS: macOS High Sierra 10.13.6
    CPU: (4) x64 Intel(R) Core(TM) i5-7600 CPU @ 3.50GHz
    Memory: 7.60 GB / 32.00 GB
  Binaries:
    Node: 10.17.0 - ~/.nvm/versions/node/v10.17.0/bin/node
    Yarn: 1.21.1 - /usr/local/bin/yarn
    npm: 6.13.4 - ~/.nvm/versions/node/v10.17.0/bin/npm
  npmPackages:
    istanbul-lib-coverage: ^3.0.0 => 3.0.0 
    istanbul-lib-hook: ^3.0.0 => 3.0.0 
    istanbul-lib-instrument: ^4.0.0 => 4.0.0 
    istanbul-lib-processinfo: ^2.0.2 => 2.0.2 
    istanbul-lib-report: ^3.0.0 => 3.0.0 
    istanbul-lib-source-maps: ^4.0.0 => 4.0.0 
    istanbul-reports: ^3.0.0 => 3.0.0 
    source-map-support: ^0.5.16 => 0.5.16

The text was updated successfully, but these errors were encountered:

AndrewFinlay · 2019-12-30T02:39:30Z

Example of a passing and failing test run one after the other with no changes to the source...

iMac-3:nyc andrewf$ npm t

> nyc@15.0.0 pretest /Users/andrewf/work/Github_projects/nyc
> npm run lint && npm run clean && npm run instrument


> nyc@15.0.0 lint /Users/andrewf/work/Github_projects/nyc
> standard


> nyc@15.0.0 clean /Users/andrewf/work/Github_projects/nyc
> node ./npm-run-clean.js


> nyc@15.0.0 instrument /Users/andrewf/work/Github_projects/nyc
> node ./build-self-coverage.js


> nyc@15.0.0 test /Users/andrewf/work/Github_projects/nyc
> tap

 PASS  test/add-all-files.js 13 OK 2s
 PASS  test/cache.js 2 OK 4s
 PASS  test/config-override.js 3 OK 1s
 PASS  test/config.js 15 OK 84.54ms
 PASS  test/cwd.js 4 OK 63.739ms
 PASS  test/eager.js 6 OK 1s
 PASS  test/instrument.js 64 OK 9s
 PASS  test/issue-190.js 4 OK 4s
 FAIL  test/nyc-integration.js
 ✖ should be equal

  test/nyc-integration.js                                             
  295 |   t.equal(status, 1)                                          
  296 |   t.equal(stdout, '')                                         
> 297 |   t.equal(stderr, help.stdout)                                
      | ----^                                                         
  298 | })                                                            
  299 |                                                               
  300 | t.test('handles --clean / --no-clean properly', async t => {  

  --- expected                                                                     
  +++ actual                                                                       
  @@ -111,5 +111,8 @@                                                              
     nyc.js npm test                           instrument your tests with coverage 
     nyc.js --require @babel/register npm      instrument your tests with coverage 
     test                                      and transpile with Babel            
     nyc.js report --reporter=text-lcov        output lcov report after running    
  -                                                                                
  +                                            your tests                          
  +                                                                                
  +visit https://git.io/vHysA for list of available reporters                      
  +                                                                                

  test: test/nyc-integration.js nyc displays help to stderr when it doesn't know what
    to do
  stack: |
    Test.t.test (test/nyc-integration.js:297:5)

 FAIL  test/nyc-integration.js 1 failed of 221 14s
 ✖ should be equal

 PASS  test/parser-plugins.js 6 OK 835.683ms
 PASS  test/process-args.js 5 OK 26.982ms
 PASS  test/processinfo.js 21 OK 2s
 PASS  test/report.js 3 OK 1s
 PASS  test/should-instrument.js 23 OK 98.925ms
 PASS  test/source-map-support.js 2 OK 441.032ms
 PASS  test/temp-dir.js 15 OK 1s
 PASS  test/tsc.js 6 OK 1s
 PASS  test/wrap.js 12 OK 577.593ms

                         
  🌈 SUMMARY RESULTS 🌈  
                         

 FAIL  test/nyc-integration.js 1 failed of 221 14s
 ✖ should be equal

Suites:   1 failed, 17 passed, 18 of 18 completed
Asserts:  1 failed, 424 passed, of 425
Time:     47s
npm ERR! Test failed.  See above for more details.
iMac-3:nyc andrewf$ npm t

> nyc@15.0.0 pretest /Users/andrewf/work/Github_projects/nyc
> npm run lint && npm run clean && npm run instrument


> nyc@15.0.0 lint /Users/andrewf/work/Github_projects/nyc
> standard


> nyc@15.0.0 clean /Users/andrewf/work/Github_projects/nyc
> node ./npm-run-clean.js


> nyc@15.0.0 instrument /Users/andrewf/work/Github_projects/nyc
> node ./build-self-coverage.js


> nyc@15.0.0 test /Users/andrewf/work/Github_projects/nyc
> tap

 PASS  test/add-all-files.js 13 OK 3s
 PASS  test/cache.js 2 OK 5s
 PASS  test/config-override.js 3 OK 1s
 PASS  test/config.js 15 OK 86.122ms
 PASS  test/cwd.js 4 OK 59.56ms
 PASS  test/eager.js 6 OK 1s
 PASS  test/instrument.js 64 OK 11s
 PASS  test/issue-190.js 4 OK 4s
 PASS  test/nyc-integration.js 221 OK 14s
 PASS  test/parser-plugins.js 6 OK 787.448ms
 PASS  test/process-args.js 5 OK 32.526ms
 PASS  test/processinfo.js 21 OK 2s
 PASS  test/report.js 3 OK 1s
 PASS  test/should-instrument.js 23 OK 111.272ms
 PASS  test/source-map-support.js 2 OK 417.408ms
 PASS  test/temp-dir.js 15 OK 1s
 PASS  test/tsc.js 6 OK 1s
 PASS  test/wrap.js 12 OK 798.066ms

                         
  🌈 SUMMARY RESULTS 🌈  
                         

Suites:   18 passed, 18 of 18 completed
Asserts:  425 passed, of 425
Time:     51s

> nyc@15.0.0 posttest /Users/andrewf/work/Github_projects/nyc
> npm run report


> nyc@15.0.0 report /Users/andrewf/work/Github_projects/nyc
> node ./bin/nyc report --temp-dir ./.self_coverage/ -r text -r lcov

-----------------------|---------|----------|---------|---------|-------------------
File                   | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
-----------------------|---------|----------|---------|---------|-------------------
All files              |     100 |      100 |     100 |     100 |                   
 nyc                   |     100 |      100 |     100 |     100 |                   
  index.js             |     100 |      100 |     100 |     100 |                   
 nyc/bin               |     100 |      100 |     100 |     100 |                   
  nyc.js               |     100 |      100 |     100 |     100 |                   
  wrap.js              |     100 |      100 |     100 |     100 |                   
 nyc/lib               |     100 |      100 |     100 |     100 |                   
  config-util.js       |     100 |      100 |     100 |     100 |                   
  fs-promises.js       |     100 |      100 |     100 |     100 |                   
  hash.js              |     100 |      100 |     100 |     100 |                   
  process-args.js      |     100 |      100 |     100 |     100 |                   
  register-env.js      |     100 |      100 |     100 |     100 |                   
  source-maps.js       |     100 |      100 |     100 |     100 |                   
  wrap.js              |     100 |      100 |     100 |     100 |                   
 nyc/lib/commands      |     100 |      100 |     100 |     100 |                   
  check-coverage.js    |     100 |      100 |     100 |     100 |                   
  helpers.js           |     100 |      100 |     100 |     100 |                   
  instrument.js        |     100 |      100 |     100 |     100 |                   
  merge.js             |     100 |      100 |     100 |     100 |                   
  report.js            |     100 |      100 |     100 |     100 |                   
 nyc/lib/instrumenters |     100 |      100 |     100 |     100 |                   
  istanbul.js          |     100 |      100 |     100 |     100 |                   
  noop.js              |     100 |      100 |     100 |     100 |                   
-----------------------|---------|----------|---------|---------|-------------------

AndrewFinlay · 2019-12-30T03:25:17Z

In case it helps, the pass/fail rate I'm seeing is roughly 50/50. The tests fail in the same way when using Node v12.14.0. I also can't get nyc --temp-dir myTestTempDir --help to output the ~~short~~ problematic help message when run from the terminal.

AndrewFinlay · 2019-12-30T05:43:06Z

After doing a bit of digging it looks like the issue is in yargs, as a call to nyc --help will hit process.exit(code) on line 978 of yargs.js and process.exit() doesn't need to wait for the stdout/stderr buffers to be flushed before exiting. The other nyc command in the test doesn't take that path and instead gets its help output by calling yargs.showHelp() before exiting normally, so it doesn't run into the same problem.

coreyfarrell · 2019-12-30T16:02:27Z

Thank you for digging into this. It sounds like yargs needs to be patched to use synchronous output fs.writeSync instead of console.log / console.error?

AndrewFinlay · 2019-12-31T00:46:14Z

I tried the fs.writeSync solution but I couldn't get it to work reliably. It looks like the best solution would be for yargs to not use process.exit(code) and exit normally after setting the desired exit code. I've created an issue on yargs that's linked above.

For now I'm just going to comment out the test locally

AndrewFinlay · 2020-01-01T23:53:25Z

I've spent a little more time with this one and can give you a pretty definitive description of what's going on although I'm not sure what the best solution is.

yargs uses undocumented node features to set blocking mode on stdout & stderr if they are TTY connections. When nyc is running its problematic test it spawns a nyc instance in a child process and assigns pipes to stdout & stderr that aren't TTY. This means that yargs can't set blocking mode and can't guarantee that all output written to stdout & stderr will be delivered.

In most use cases this will never be a problem and both nyc and yargs will work as intended.

I'm not sure what the best solution is here, it still seems like a yargs bug to me, I can't see why yargs doesn't support non-TTY stdout & stderr but there could be a good reason for it. It may be possible to create a hacky work around in nyc to support testing, maybe by spawning a test wrapper first that sets process.std<x>.isTTY then spawns nyc with inherited stdio.

coreyfarrell · 2020-01-02T14:44:57Z

I wonder if yargs could set blocking mode for non-TTY.

One solution to this test might be to loosen the comparison so we allow the test to pass if we get truncated output. The main point of the test is to verify that the help is dumped under the two conditions, if I remember correctly comparing the output was simply an easy way to accomplish this. I tend to agree with you that this is unlikely to be an issue in the wild. In theory we could do some trickery of creating a PTY but I'd rather not go that far for this one test.

AndrewFinlay · 2020-01-19T21:42:28Z

I can't see this being fixed in yargs any time soon, so loosening the test is probably a good idea.
I'm thinking checking that the possibly truncated output is a substring of the full help output.

This change allows for flaky output of help information from `yargs` to `stderr` when running `nyc` as a child process. More information can be found [here](istanbuljs#1259).

This change allows for flaky output of help information from `yargs` to `stderr` when running `nyc` as a child process. Fixes #1259

AndrewFinlay changed the title ~~Flaky test in nyc~~ Flaky test in nyc's test suite Dec 30, 2019

AndrewFinlay mentioned this issue Dec 31, 2019

Flaky output from --help yargs/yargs#1519

Open

AndrewFinlay mentioned this issue Jan 20, 2020

fix: A test where nyc output help text to stderr was flaky #1269

Merged

coreyfarrell closed this as completed in #1269 Apr 1, 2020

coreyfarrell pushed a commit that referenced this issue Apr 1, 2020

chore: A test where nyc output help text to stderr was flaky (#1269)

befbf08

This change allows for flaky output of help information from `yargs` to `stderr` when running `nyc` as a child process. Fixes #1259

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Flaky test in nyc's test suite #1259

Flaky test in nyc's test suite #1259

AndrewFinlay commented Dec 30, 2019 •

edited

Loading

AndrewFinlay commented Dec 30, 2019 •

edited

Loading

AndrewFinlay commented Dec 30, 2019 •

edited

Loading

AndrewFinlay commented Dec 30, 2019

coreyfarrell commented Dec 30, 2019

AndrewFinlay commented Dec 31, 2019

AndrewFinlay commented Jan 1, 2020

coreyfarrell commented Jan 2, 2020

AndrewFinlay commented Jan 19, 2020

Flaky test in nyc's test suite #1259

Flaky test in nyc's test suite #1259

Comments

AndrewFinlay commented Dec 30, 2019 • edited Loading

Link to bug demonstration repository

Expected Behavior

Observed Behavior

Troubleshooting steps

Environment Information

AndrewFinlay commented Dec 30, 2019 • edited Loading

AndrewFinlay commented Dec 30, 2019 • edited Loading

AndrewFinlay commented Dec 30, 2019

coreyfarrell commented Dec 30, 2019

AndrewFinlay commented Dec 31, 2019

AndrewFinlay commented Jan 1, 2020

coreyfarrell commented Jan 2, 2020

AndrewFinlay commented Jan 19, 2020

AndrewFinlay commented Dec 30, 2019 •

edited

Loading

AndrewFinlay commented Dec 30, 2019 •

edited

Loading

AndrewFinlay commented Dec 30, 2019 •

edited

Loading