Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: Reporter interface typing is too strong? #14427

Closed
liamjones opened this issue Aug 18, 2023 · 10 comments · Fixed by #14433 or #14435
Closed

[Bug]: Reporter interface typing is too strong? #14427

liamjones opened this issue Aug 18, 2023 · 10 comments · Fixed by #14433 or #14435

Comments

@liamjones
Copy link

Version

29.6.2

Steps to reproduce

  1. Write a custom reporter in JavaScript that only implements onRunComplete, as documented here: https://jestjs.io/docs/configuration/#custom-reporters.
  2. Check reporter works fine with Jest.
  3. Migrate JS to TypeScript and implement Reporter (see https://codesandbox.io/s/sharp-babbage-6v54kc?file=/src/index.ts for example)

Expected behavior

No TypeScript errors

Actual behavior

TypeScript error:

Class 'CustomReporter' incorrectly implements interface 'Reporter'.
   Type 'CustomReporter' is missing the following properties from type 'Reporter': onRunStart, getLastErrorts(2420)

Additional context

No response

Environment

System:
    OS: macOS 13.5.1
    CPU: (16) x64 Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
  Binaries:
    Node: 18.17.0 - ~/Library/Caches/fnm_multishells/11306_1692352089946/bin/node
    Yarn: 1.22.19 - ~/Library/Caches/fnm_multishells/11306_1692352089946/bin/yarn
    npm: 9.8.1 - ~/Library/Caches/fnm_multishells/11306_1692352089946/bin/npm
  npmPackages:
    jest: ^29.2.1 => 29.6.2
@SimenB
Copy link
Member

SimenB commented Aug 18, 2023

They're all optional in reality, so PR very much welcome 🙂

export interface Reporter {
readonly onTestResult?: (
test: Test,
testResult: TestResult,
aggregatedResult: AggregatedResult,
) => Promise<void> | void;
readonly onTestFileResult?: (
test: Test,
testResult: TestResult,
aggregatedResult: AggregatedResult,
) => Promise<void> | void;
/**
* Called before running a spec (prior to `before` hooks)
* Not called for `skipped` and `todo` specs
*/
readonly onTestCaseStart?: (
test: Test,
testCaseStartInfo: Circus.TestCaseStartInfo,
) => Promise<void> | void;
readonly onTestCaseResult?: (
test: Test,
testCaseResult: TestCaseResult,
) => Promise<void> | void;
readonly onRunStart: (
results: AggregatedResult,
options: ReporterOnStartOptions,
) => Promise<void> | void;
readonly onTestStart?: (test: Test) => Promise<void> | void;
readonly onTestFileStart?: (test: Test) => Promise<void> | void;
readonly onRunComplete: (
testContexts: Set<TestContext>,
results: AggregatedResult,
) => Promise<void> | void;
readonly getLastError: () => Error | void;
}

@liamjones
Copy link
Author

They're all optional in reality

Even onRunComplete? The docs say:

"Custom reporter module must export a class that takes globalConfig, reporterOptions and reporterContext as constructor arguments and implements at least onRunComplete() method"

PR very much welcome

Sure! I can do one for the docs too if they're not accurate

@mrazauskas
Copy link
Contributor

Hooks of the reporters are called here: jest/packages/jest-core/src/ReporterDispatcher.ts.

Looks like all are optional. Just as Simen said already.

@SimenB
Copy link
Member

SimenB commented Aug 18, 2023

PR very much welcome

Sure! I can do one for the docs too if they're not accurate

yes please 😀

@quantumflo
Copy link

Hi, Is someone already creating a PR?
If not I can work on this issue.

@mrazauskas
Copy link
Contributor

Typings got fixed. It would be useful to amend documentation as well.

@mrazauskas mrazauskas reopened this Aug 21, 2023
@quantumflo
Copy link

@mrazauskas I can contribute but is it documentation for the hook?

@mrazauskas
Copy link
Contributor

@quantumflo Thanks for your interested! I took a better look at the problem and understood that the change is more involved than I thought initially. It was faster to open a PR (see #14435) than to explain the intention.

@liamjones
Copy link
Author

@quantumflo Thanks for picking this up, I was on holiday last week 😎

@github-actions
Copy link

This issue 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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants