the same context for addEventHandler from jest-circus/src/state.ts

[satanTime]

🐛 Bug Report

The problem is that I need to distinguish scopes on every beforeAll and beforeEach call.
However, adding a listener on addEventHandler doesn't bring the desired effect, because the listener is added to a different context and hasn't been ever called.

With jasmine it was quite easy:

jasmine.getEnv().addReporter(reporterStack);

And would be great to have the same behavior in jest-circus.

To Reproduce

This is the test and desired code: https://github.com/satanTime/jest-circus-hooks/blob/master/src/app/app.component.spec.ts#L7-L21

Steps to reproduce the behavior:

• clone https://github.com/satanTime/jest-circus-hooks
• npm install
• npm run test
• failure

> jest -w 1 --config jest.js

 FAIL  src/app/app.component.spec.ts
  main block
    ✕ uses listener (2 ms)
    sub block
      ✓ simulates positive (3 ms)

• edit jest.ts and uncomment testRunner: 'jest-jasmine2',
• npm run test
• no failure

This is the place I want to add my callback to.

https://github.com/facebook/jest/blob/master/packages/jest-circus/src/state.ts#L14

Expected behavior

addEventHandler register a callback properly and the callback is triggered on events.

For example, when I change the source code in node_modules to:

const eventHandlers = window.eventHandlers || [_eventHandler.default, _formatNodeAssertErrors.default];
window.eventHandlers = eventHandlers;

it works correctly.

Proposed PR with a fix

The PR is here #11529

envinfo

  System:
    OS: macOS 11.4
    CPU: (16) x64 Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
  Binaries:
    Node: 12.22.1 - /opt/local/bin/node
    Yarn: 1.22.10 - /opt/local/bin/yarn
    npm: 6.14.13 - /opt/local/bin/npm
  npmPackages:
    jest: 27.0.3 => 27.0.3

[klizter]

Bump!

[satanTime]

Hi there, I'll prepare a min demo today / next days.

[satanTime]

Added a min repo with steps.

[satanTime]

I've created a PR with a fix.

[klizter]

Great work @satanTime

[satanTime]

Hi @SimenB,

I hope this finds you well.

There are 9 upvotes already, could you take a look at the issue and its fix when you have time?

Thank you in advance!

[satanTime]

Hi @SimenB, could you take a look at the issue and the related PR?

Thank you in advance!

[satanTime]

Hi @SimenB,

there are 40 upvotes and a proposed fix.
Might you take a look when you have time or forward me to a person who could take care about code review?

Thank you in advance.

[satanTime]

Hi @SimenB,

I hope this finds you well.

However, it is a pity to see how you treat your community.
There are 60 upvotes within less than 2 months.
The issue is one of the most upvoted...

...and zero feedback from your side.

How should open source encourage devs to participate in it, when an easy fix stumbles over а disregard from the repo maintainers?

[sigveio]

To be fair - the reason it's one of the most upvoted is probably because people are being prompted by ng-mocks to go and do so. So the upvote count doesn't necessarily reflect the real impact of this issue on the community as a whole, or how urgent it is to fix compared to other things.

I understand that it's frustrating to put time and effort into proposing a fix without getting response in a timely manner. But I feel the tone in your last comment is unnecessarily sharp/negative. Warranted or not, I don't think it will help at all.

Simen (and I'm sure others) are doing a tremendous effort to bring us awesome tooling here, and I bet there are a lot of different things demanding their attention every single day. We should try to remain respectful to each other and not add to the burden they are already carrying.

It's also worth keeping in mind that many have had a demanding year and a half with the pandemic. And that it's currently vacation time in much of Europe. So expect extra delays.

Meanwhile... perhaps there are some qualified individuals amongst those 60 who could take the time to review the PR and not just upvote? It might help move it along when someone with access to merge have time to look at it.

✌️ @ ❤️

[satanTime]

Hi @sigveio, you are welcome to review the PR.

[szakharchenko]

I can only speak for myself, but I stumbled upon this PR when searching for a way to add test event handlers, and it looked much like how I would have done it. For a reasonably sized package, I would have just forked it privately, exposed the functionality and never bothered to search in the first place, and ultimately would never know people taking voting advice from NPM packages was a thing, so maybe it was all for the best;).

[jaytonic]

Any chance the PR can be approved? Not sure to understand what is the blocking point since last summer?

[satanTime]

The main point is that maintainers don't understand the fix and don't have time to understand it.

Although, I thought, it is an easy one.

[mircoservices]

For anyone interested here a workaround using patch-package and jest-circus@28.1.3:

Add file patches/jest-circus+28.1.3.patch to your workspace:

diff --git a/node_modules/jest-circus/build/index.d.ts b/node_modules/jest-circus/build/index.d.ts
index adab032..a1ed485 100644
--- a/node_modules/jest-circus/build/index.d.ts
+++ b/node_modules/jest-circus/build/index.d.ts
@@ -113,6 +113,10 @@ export declare const setState: (state: Circus.State) => Circus.State;
 
 export declare type State = Circus.State;
 
+export declare const addEventHandler: (handler: Circus.EventHandler) => void;
+
+export declare const removeEventHandler: (handler: Circus.EventHandler) => void;
+
 declare const test_2: Global.It;
 export {test_2 as test};
 
diff --git a/node_modules/jest-circus/build/index.js b/node_modules/jest-circus/build/index.js
index 1c848bf..7e9c6d4 100644
--- a/node_modules/jest-circus/build/index.js
+++ b/node_modules/jest-circus/build/index.js
@@ -35,6 +35,18 @@ Object.defineProperty(exports, 'setState', {
     return _state.setState;
   }
 });
+Object.defineProperty(exports, 'addEventHandler', {
+  enumerable: true,
+  get: function () {
+    return _state.addEventHandler;
+  }
+});
+Object.defineProperty(exports, 'removeEventHandler', {
+  enumerable: true,
+  get: function () {
+    return _state.removeEventHandler;
+  }
+});
 exports.test = void 0;
 
 var _jestEach = require('jest-each');
diff --git a/node_modules/jest-circus/build/state.js b/node_modules/jest-circus/build/state.js
index 528b774..f194111 100644
--- a/node_modules/jest-circus/build/state.js
+++ b/node_modules/jest-circus/build/state.js
@@ -32,7 +32,7 @@ function _interopRequireDefault(obj) {
  * This source code is licensed under the MIT license found in the
  * LICENSE file in the root directory of this source tree.
  */
-const eventHandlers = [_eventHandler.default, _formatNodeAssertErrors.default];
+global[_types.EVENT_HANDLERS] = global[_types.EVENT_HANDLERS] || [_eventHandler.default, _formatNodeAssertErrors.default];
 const ROOT_DESCRIBE_BLOCK_NAME = 'ROOT_DESCRIBE_BLOCK';
 exports.ROOT_DESCRIBE_BLOCK_NAME = ROOT_DESCRIBE_BLOCK_NAME;
 
@@ -74,7 +74,7 @@ const setState = state => (global[_types.STATE_SYM] = state);
 exports.setState = setState;
 
 const dispatch = async event => {
-  for (const handler of eventHandlers) {
+  for (const handler of global[_types.EVENT_HANDLERS]) {
     await handler(event, getState());
   }
 };
@@ -82,7 +82,7 @@ const dispatch = async event => {
 exports.dispatch = dispatch;
 
 const dispatchSync = event => {
-  for (const handler of eventHandlers) {
+  for (const handler of global[_types.EVENT_HANDLERS]) {
     handler(event, getState());
   }
 };
@@ -90,7 +90,14 @@ const dispatchSync = event => {
 exports.dispatchSync = dispatchSync;
 
 const addEventHandler = handler => {
-  eventHandlers.push(handler);
+  global[_types.EVENT_HANDLERS].push(handler);
 };
 
 exports.addEventHandler = addEventHandler;
+
+const removeEventHandler = handler => {
+  const i = global[_types.EVENT_HANDLERS].indexOf(handler);
+  global[_types.EVENT_HANDLERS].splice(i, 1);
+};
+
+exports.removeEventHandler = removeEventHandler;
diff --git a/node_modules/jest-circus/build/types.js b/node_modules/jest-circus/build/types.js
index 70116de..5f552fb 100644
--- a/node_modules/jest-circus/build/types.js
+++ b/node_modules/jest-circus/build/types.js
@@ -25,3 +25,6 @@ const TEST_TIMEOUT_SYMBOL = Symbol.for('TEST_TIMEOUT_SYMBOL');
 exports.TEST_TIMEOUT_SYMBOL = TEST_TIMEOUT_SYMBOL;
 const LOG_ERRORS_BEFORE_RETRY = Symbol.for('LOG_ERRORS_BEFORE_RETRY');
 exports.LOG_ERRORS_BEFORE_RETRY = LOG_ERRORS_BEFORE_RETRY;
+
+const EVENT_HANDLERS = Symbol.for('EVENT_HANDLERS');
+exports.EVENT_HANDLERS = EVENT_HANDLERS;

[github-actions]

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

[Proper-Job]

We're still having to patch jest circus like @mircoba suggested: #11483 (comment)