jestjs / jest

Delightful JavaScript Testing.
https://jestjs.io
MIT License
43.95k stars 6.41k forks source link

[Bug]: Different reporter order leads to `coverageMap` missing in custom reporter starting in Jest 28 #14052

Open danieldiekmeier opened 1 year ago

danieldiekmeier commented 1 year ago

Version

29.5.0

Steps to reproduce

  1. Clone repo: https://github.com/danieldiekmeier/jest-custom-reporter-reproduction
  2. npm i
  3. npm test

In the output and in the reporter.js file, you can see that I had to use a setInterval to keep my custom reporter running until after the CoverageReporter is finished collecting the coverage. In Jest 27, this wasn't necessary – by the time my custom reporter ran, the CoverageReporter was already finished.

Expected behavior

I expect my custom reporters to run last (like they did in Jest 27). Alternatively, it'd be fine if I could influence the order somehow.

Actual behavior

The coverage reporter runs after my custom reporter, making it impossible to use its results in custom reporters.

Additional context

This is the new code in Jest >=28.0.0, which registers the CoverageReporter after the custom reporter: https://github.com/facebook/jest/blob/v28.0.0/packages/jest-core/src/TestScheduler.ts#L329-L364

Up until Jest 27.5.1, the code looked like this, registering the CoverageReporter first: https://github.com/facebook/jest/blob/v27.5.1/packages/jest-core/src/TestScheduler.ts#L349-L380

This is probably a duplicate of the automatically closed #13479.

Environment

System:
  OS: macOS 13.3
  CPU: (10) arm64 Apple M1 Pro
Binaries:
  Node: 16.19.1 - ~/.asdf/installs/nodejs/16.19.1/bin/node
  npm: 9.6.0 - ~/.asdf/plugins/nodejs/shims/npm
npmPackages:
  jest: ^29.5.0 => 29.5.0
mrazauskas commented 1 year ago

Thanks for reminding this issue. It was me who changed order of reporters some time ago.

Reverting it would be a breaking change. I think the problem is not the order, but the fact one of built-in reporters is adding coverage data to aggregatedResult. It sounds like better idea to populate the coverageMap before calling the onRunComplete method of each reporter. For example, here:

https://github.com/facebook/jest/blob/257f250a977264656eca386629c7ce3b6481f21b/packages/jest-core/src/ReporterDispatcher.ts#L83-L92

I will try to put this together. That is a breaking change as well, so it has to wait till next major.

github-actions[bot] commented 1 year ago

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 30 days.

danieldiekmeier commented 1 year ago

This can't be stale, it's still in triage!

github-actions[bot] commented 1 year ago

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 30 days.

danieldiekmeier commented 1 year ago

Still needs triage!

github-actions[bot] commented 1 year ago

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 30 days.

SimenB commented 10 months ago

@mrazauskas are you still up for sending a PR for this? 🙂

mrazauskas commented 10 months ago

Yes. Not this week, but in general I would like to do this.

SimenB commented 7 months ago

@mrazauskas ping 😀

aegSol commented 5 months ago

Any news on this?

mrazauskas commented 5 months ago

Uff.. I can’t find time unfortunately. Feel free to take it over.

i-dont-know-javascript commented 4 months ago

@danieldiekmeier Hi! Have you got better solution than your current one? Are you going to open your PR?

danieldiekmeier commented 4 months ago

@i-dont-know-javascript Hello! My solution was to stop using Jest. I never planned to open a PR about this, but even getting this bug triaged/acknowledged took literally MONTHS. I no longer care either way.

Schweinepriester commented 4 months ago

I, at @danieldiekmeier​s previous company (Hi Daniel! 👋), still care though 😅