search

avajs / ava

Node.js test runner that lets you develop with confidence 🚀
MIT License
20.74k stars 1.41k forks source link

Reporters shouldn't assume the only error when `--match` is used is that it didn't match any tests #2191

Closed rauschma closed 4 years ago

rauschma commented 5 years ago

Steps to reproduce (AVA: 2.2.0, macOS Mojave, Node.js 12.6):

Interaction 1: bug (error output is hidden)

$ npm t ./exercises/strings/remove_extension_test.mjs

  ✖ Couldn't find any matching tests

Interaction 2: desired output

$ npm t ./exercises/strings/remove_extension_test.mjs  -- -v

  Uncaught exception in exercises/strings/remove_extension_test.mjs

  Error: Cannot find module './remove_extension.mjs'
  Require stack:
  - /Users/rauschma/tmp/impatient-js-preview-code/exercises/strings/remove_extension_test.mjs
  - /Users/rauschma/tmp/impatient-js-preview-code/node_modules/ava/lib/worker/subprocess.js

  ✖ exercises/strings/remove_extension_test.mjs exited with a non-zero exit code: 1
  ✖ Couldn't find any matching tests
novemberborn commented 5 years ago

Yea that's pretty bizarre. It should work the same as without --match, regardless of the -v flag.

vmlf01 commented 5 years ago

To reproduce this, I had to run with npx ava ./exercises/strings/remove_extension_test.mjs -m removeExtension (note the additional -m removeExtension option), which I guess is missing in the instructions above?

There is an early return in the block at https://github.com/avajs/ava/blob/master/lib/reporters/mini.js#L382, which prevents further error details from being output when running without -v flag.

if (this.matching && this.stats.selectedTests === 0) {
    this.lineWriter.writeLine(colors.error(`${figures.cross} Couldn't find any matching tests`));
    this.lineWriter.writeLine();
    return;
}

Maybe that could be moved further down the method, but I don't know how that will affect output for other cases.

novemberborn commented 5 years ago

To reproduce this, I had to run with npx ava ./exercises/strings/remove_extension_test.mjs -m removeExtension (note the additional -m removeExtension option), which I guess is missing in the instructions above?

In the reproduction, it's in AVA's configuration, not on the CLI.

Maybe that could be moved further down the method, but I don't know how that will affect output for other cases.

Yea we have a bunch of reporter issues. I think they're more fundamental than can be addressed in individual tickets. Will write up a larger issue instead.

novemberborn commented 5 years ago

@vmlf01 thanks for finding the root cause though!

rauschma commented 5 years ago

@vmlf01 Ah, sorry, the latest version already contains the workaround. I’ve updated the installation instructions. Now they simply remove the workaround.

novemberborn commented 4 years ago

Both due to the age of this issue, and the state of our reporters, I've decided to roll this into #2501.