feat(jest-reporters): pass `reporterContext` to custom reporter constructors as third argument #12657

mrazauskas · 2022-04-09T12:20:41Z

Summary

Promise, I was refactoring jest-worker, but noticed strange details in types of jest-reporters. Just could not leave it.

What if constructors of custom reporters alongside with their config options would also receive reporterContext?

Similarly default reporters all could receive unified reporterContext object. Currently context is passed to reporters, but is typed somewhat differently. That feels redundant.

Also this change would allow writing custom reporters similar to coverage or notify. Why not? (;

Test plan

Green CI.

mrazauskas · 2022-04-09T12:21:51Z

docs/Configuration.md


 ```json
 {
  "reporters": [
    "default",
-    ["<rootDir>/my-custom-reporter.js", {"banana": "yes", "pineapple": "no"}]
+    ["jest-junit", {"outputName": "junit-report.xml", "suiteName": "some-name"}]


Just to convince Prettier to keep this as separate line.

mrazauskas · 2022-04-09T12:26:28Z

packages/jest-core/src/TestScheduler.ts

-      return {options: this._options, path: reporter};
+      return {options: {}, path: reporter};


startRun() was passed here. Perhaps for notify reporter?.

Hm.. Possibly this branch is never called, because reporters are already normalised:

https://github.com/facebook/jest/blob/62b4bd701db378b38d360094f55fbd9ba64ca937/packages/jest-config/src/normalize.ts#L415-L429

let's remove it, probably for legacy reasons

mrazauskas · 2022-04-09T12:27:02Z

packages/jest-reporters/package.json

@@ -49,6 +47,7 @@
    "@types/istanbul-lib-source-maps": "^4.0.0",
    "@types/istanbul-reports": "^3.0.0",
    "@types/node-notifier": "^8.0.0",
+    "jest-resolve": "^28.0.0-alpha.8",


Needed for tests only.

packages/jest-reporters/src/index.ts

mrazauskas · 2022-04-09T12:31:51Z

packages/jest-reporters/src/types.ts

-export type OnTestStart = (test: Test) => Promise<void>;
-export type OnTestFailure = (
-  test: Test,
-  error: SerializableError,
-) => Promise<unknown>;
-export type OnTestSuccess = (
-  test: Test,
-  result: TestResult,
-) => Promise<unknown>;


Not used and not public exports.

mrazauskas · 2022-04-09T12:32:13Z

packages/jest-reporters/src/types.ts

-export type TestRunnerOptions = {
-  serial: boolean;
-};
-
-export type TestRunData = Array<{
-  context: Context;
-  matches: {allTests: number; tests: Array<Test>; total: number};
-}>;


Also not used.

mrazauskas · 2022-04-10T08:00:09Z

packages/jest-reporters/src/index.ts

 export type {
  AggregatedResult,
-  SnapshotSummary,


Fixed. Types for all public classes and utilities are exported. SnapshotSummary type is only used by getSnapshotSummary() util. This one is not exposed, but perhaps it should be exported together with getSnapshotStatus()? Looks helpful.

docs/Configuration.md

CHANGELOG.md

docs/Configuration.md

mrazauskas · 2022-04-11T07:27:20Z

@SimenB Documentation got improved to address your last comments.

User defined reporterOptions were already passed as second argument to reporter constructor. There is no change in jest-config. I was just refactoring docs: first all details about reporters option, last all about how to build a custom reporter. The custom reporter part must have more details. It might be better to move it to separate file, but that will be another PR.

I was thinking if options and context could be combined into one object and passed liked this from test scheduler:

new Reporter(this._globalConfig, {...options, _context: this._context})

Could work, but extra nesting is somewhat clumsy. Alternative:

new Reporter(this._globalConfig, {...options, ...this._context})

Oh, that would be dangerous. Another attempt:

new Reporter(
  this._globalConfig,
  {
    ...options,
    _firstRun: this._context.firstRun,
    _previousSuccess: this._context.previousSuccess,
    // and the rest of context in similar fashion
  }
)

Hm.. That’s alright, but what about separation of concerns? These are not user defined options. Also might not work in long term.

It seemed that this signature would be the most optimal:

constructor(globalConfig, reporterOptions, reporterContext)

Will not break with older reporters. Perhaps reporterContext could be named better? Simply context? We have TestContext around, so I though some prefix is good idea. That’s not breaking, can be changed at any time.

docs/Configuration.md

SimenB

yay!

SimenB · 2022-04-11T08:04:46Z

packages/jest-core/src/TestScheduler.ts

  } {
    if (typeof reporter === 'string') {
-      return {options: this._options, path: reporter};
+      return {options: {}, path: reporter};
    } else if (Array.isArray(reporter)) {
      const [path, options] = reporter;


add a check that tuple length is 2?

Can we leave this for another PR? Currently this method is called twice, that's nothing significant, but still unnecessary. I have an idea how to rework it, but that's another story (;

mrazauskas · 2022-04-11T14:59:18Z

packages/jest-types/src/Config.ts

@@ -411,7 +411,7 @@ export type GlobalConfig = {
  passWithNoTests: boolean;
  projects: Array<string>;
  replname?: string;
-  reporters?: Array<string | ReporterConfig>;


This should type normalized config, right? I have added tests jest-config, which show that reporters are always normalized to a tuple. Having more precise type here, makes it possible to have less logic inside TestScheduler. Last commits are added just to illustrate the solution. Perhaps it would be better to send separate PR? Feel free to revert them.

SimenB

great!

github-actions · 2022-05-13T00:10:06Z

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

feat: pass reporterContext to Reporter constructor

07a9d43

facebook-github-bot added the cla signed label Apr 9, 2022

reporterConstructor

238be92

mrazauskas commented Apr 9, 2022

View reviewed changes

mrazauskas added 5 commits April 9, 2022 15:40

add changelog entry

1a53908

fix changelog

50a8241

rebase

6b7f017

keep type reexports

deb6074

sort exports

e13963c

mrazauskas commented Apr 10, 2022

View reviewed changes

expose getSnapshotStatus and getSnapshotSummary

4afff9f

mrazauskas changed the title ~~feat(jest-reporters)!: pass reporterContext to custom reporter constructors as third argument~~ feat(jest-reporters): pass reporterContext to custom reporter constructors as third argument Apr 10, 2022

better TestSchedulerContext type?

3a80da7

SimenB reviewed Apr 10, 2022

View reviewed changes

docs/Configuration.md Outdated Show resolved Hide resolved

SimenB reviewed Apr 10, 2022

View reviewed changes

CHANGELOG.md Show resolved Hide resolved

SimenB reviewed Apr 10, 2022

View reviewed changes

docs/Configuration.md Outdated Show resolved Hide resolved

tweak docs

aed2772

mrazauskas commented Apr 11, 2022

View reviewed changes

docs/Configuration.md Outdated Show resolved Hide resolved

Update docs/Configuration.md

1e7f073

SimenB reviewed Apr 11, 2022

View reviewed changes

docs/Configuration.md Outdated Show resolved Hide resolved

SimenB approved these changes Apr 11, 2022

View reviewed changes

Update docs/Configuration.md

bbb8f97

SimenB reviewed Apr 11, 2022

View reviewed changes

mrazauskas added 3 commits April 11, 2022 17:40

fix reporters type

59ee2af

clean up logic

ee0cc73

Merge branch 'fix-reporters-type'

d1a625f

mrazauskas commented Apr 11, 2022

View reviewed changes

mrazauskas added 2 commits April 11, 2022 18:29

more test

cdb3daf

Merge branch 'fix-reporters-type' into feat-reporterContext

c331a40

SimenB approved these changes Apr 12, 2022

View reviewed changes

SimenB merged commit 8167899 into jestjs:main Apr 12, 2022

mrazauskas deleted the feat-reporterContext branch April 12, 2022 07:25

github-actions bot locked as resolved and limited conversation to collaborators May 13, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(jest-reporters): pass `reporterContext` to custom reporter constructors as third argument #12657

feat(jest-reporters): pass `reporterContext` to custom reporter constructors as third argument #12657

mrazauskas commented Apr 9, 2022

mrazauskas Apr 9, 2022

mrazauskas Apr 9, 2022 •

edited

Loading

mrazauskas Apr 9, 2022

SimenB Apr 11, 2022 •

edited

Loading

mrazauskas Apr 9, 2022

mrazauskas Apr 9, 2022

mrazauskas Apr 9, 2022

mrazauskas Apr 10, 2022

mrazauskas commented Apr 11, 2022 •

edited

Loading

SimenB left a comment

SimenB Apr 11, 2022

mrazauskas Apr 11, 2022

mrazauskas Apr 11, 2022

SimenB Apr 12, 2022

SimenB left a comment

github-actions bot commented May 13, 2022

		return {options: this._options, path: reporter};
		return {options: {}, path: reporter};

feat(jest-reporters): pass reporterContext to custom reporter constructors as third argument #12657

feat(jest-reporters): pass reporterContext to custom reporter constructors as third argument #12657

Conversation

mrazauskas commented Apr 9, 2022

Summary

Test plan

Choose a reason for hiding this comment

mrazauskas Apr 9, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SimenB Apr 11, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrazauskas commented Apr 11, 2022 • edited Loading

SimenB left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SimenB left a comment

Choose a reason for hiding this comment

github-actions bot commented May 13, 2022

feat(jest-reporters): pass `reporterContext` to custom reporter constructors as third argument #12657

feat(jest-reporters): pass `reporterContext` to custom reporter constructors as third argument #12657

mrazauskas Apr 9, 2022 •

edited

Loading

SimenB Apr 11, 2022 •

edited

Loading

mrazauskas commented Apr 11, 2022 •

edited

Loading