Manual rerun doesn't work when verbose mode is off

[pcdevil]

Hello,
I'm using AVA with watch mode, and noticed the manual rerun doesn't work if verbose mode is off in the config.

Setup

related package.json part:

{
  /* ... */
  "scripts": {
    "test": "ava --config ava.config.verbose-off.js",
    "test:verbose": "ava --config ava.config.verbose-on.js"
  },
  "devDependencies": {
    "ava": "^3.7.0"
  }
}

ava.config.verbose-off.js:

export default {
        verbose: false,
        watch: true,
}

ava.config.verbose-on.js:

export default {
        verbose: true,
        watch: true,
}

I made a showcase repository to give a quick example of the problem: https://github.com/pcdevil/ava-verbose-test (after cloning, npm install is necessary)

Expected behaviour

When I run the npm run test command the tests can be manually rerun by typing r + <Enter>.

Actual behaviour

The documented feature doesn't triggers AVA to rerun the tests, while using the npm run test:verbose task works properly.

Potential cause

I see AVA uses the MiniReporter and VerboseReporter respectively, and while Watcher.observeStdin() registers callback for the "data" event on stdin, it will never be called somehow when the MiniReporter is used.

Due to the lack of domain knowledge I didn't dig deeper, but I see the two reporters have very similar codebase and I suspect a modification was not applied on both classes which made the MiniReporter behave incorrectly.

The amount of code duplication and the potential cause leads me to believe a targeted refactor would be beneficial to eliminate future discrepancies.
With some guidance from a maintainer I'd happily work on this, but I didn't want to just jump and into and endless pit :)

[novemberborn]

Nice find @pcdevil!

Hopefully we can do some more copy/pasting to solve this?

The amount of code duplication and the potential cause leads me to believe a targeted refactor would be beneficial to eliminate future discrepancies.
With some guidance from a maintainer I'd happily work on this, but I didn't want to just jump and into and endless pit :)

That'd be amazing. #2217 summarizes an approach. If it's an incentive, this would help solve many other issues that have bounties on them, see this list: https://github.com/avajs/ava/issues?q=is%3Aissue+is%3Aopen+sort%3Aupdated-desc+label%3Ascope%3Areporters+label%3A%22%3Adollar%3A+Funded+on+Issuehunt%22

[pcdevil]

@novemberborn thanks for the kind response!

Hopefully we can do some more copy/pasting to solve this?

I'd prefer another approach :)

I see there were interest from others for #2217 - can I consider it is up for grabs for now?
I have some spare time in the upcoming days, I'd create a draft PR for early feedback and see where I can go with it :)

[novemberborn]

Hopefully we can do some more copy/pasting to solve this?

I'd prefer another approach :)

Sure, but copy/pasting will at least get a fix out sooner.

I see there were interest from others for #2217 - can I consider it is up for grabs for now?
I have some spare time in the upcoming days, I'd create a draft PR for early feedback and see where I can go with it :)

Sounds great. If you comment on that issue I can assign it to you.

[pcdevil]

Sure, but copy/pasting will at least get a fix out sooner.

I will see how I can handle - I'm just generally against "ohh, I just copy this piece of code and it will be fine" method, but yes, you are right, the sooner fix the better!

Sounds great. If you comment on that issue I can assign it to you.

Done!

I'd separate the work with the two issue to reduce the footprint of the fix itself and presumably make it more rapidly.
Do you agree with this? I'm sensing by your previous comment you do, but want to check.

[novemberborn]

I'd separate the work with the two issue to reduce the footprint of the fix itself and presumably make it more rapidly.
Do you agree with this? I'm sensing by your previous comment you do, but want to check.

Yea. Rebuilding the reporters is a lot of work.