-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
the same context for addEventHandler from jest-circus/src/state.ts
#11483
Comments
Bump! |
Hi there, I'll prepare a min demo today / next days. |
Added a min repo with steps. |
I've created a PR with a fix. |
Great work @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! |
Hi @SimenB, could you take a look at the issue and the related PR? Thank you in advance! |
Hi @SimenB, there are 40 upvotes and a proposed fix. Thank you in advance. |
Hi @SimenB, I hope this finds you well. However, it is a pity to see how you treat your community. ...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? |
To be fair - the reason it's one of the most upvoted is probably because people are being prompted by 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. ✌️ @ ❤️ |
Hi @sigveio, you are welcome to review the PR. |
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;). |
Any chance the PR can be approved? Not sure to understand what is the blocking point since last summer? |
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. |
For anyone interested here a workaround using patch-package and Add file
|
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. |
We're still having to patch jest circus like @mircoba suggested: #11483 (comment) |
🐛 Bug Report
The problem is that I need to distinguish scopes on every
beforeAll
andbeforeEach
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: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:
jest.ts
and uncommenttestRunner: 'jest-jasmine2',
This is the place I want to add my callback to.
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:it works correctly.
Proposed PR with a fix
The PR is here #11529
envinfo
The text was updated successfully, but these errors were encountered: