Improve jest-circus interop with IntelliJ

[G-Rath]

🚀 Feature Proposal

This is a split off #6295 after talking to @segrey.

While jest-circus has a lot of cool things, and seems fairly stable & feature-full, it doesn't seem to provide the same level of detail that can be obtained by using jasmine, which means IntelliJ can't enhance it's test reporter subsystem to do cool things like showing inline diffs & editor-level highlighting of erroring matchers (i.e red squiggles & popup messages a la eslint & ts).

It would be great to scope out what's required to allow IntelliJ to support jest-circus in the same manner as it does jasmine2.

Motivation

Interop is cool, and it'll make migration to jest-circus even better for JetBrains users, since they'll not notice a difference in how their IDE behaves.

Related to #8840

[G-Rath]

Comments from original thread:

#6295 (comment)

@segrey:

@G-Rath Thanks for the heads up! Yep, IntelliJ Jest integration uses a Jasmine reporter to get access to more fine-grained events about tests execution. For example, this allows to have "Click to see difference" link that shows a diff dialog for expected/actual values for failed assertions.
Since the Jasmine reporter consumes Jasmine Reporter API, it can be attached to jest-jasmine2 only.

I hope that Jest reporters API (--reporters) will be improved one day (#8840) to eliminate the need in IntelliJ Jasmine reporter. If this is not going to happen before jest-circus becomes the default runner, then probably IntelliJ should support jest-circus in the same way as it supports jest-jasmine2 unless there are other options?

My understanding is that jest-circus is completely backwards compatible, so in this case all that should be required is changing that string - while jest-circus might have new API endpoints that IntelliJ/WebStorm could use, treating jest-circus as jest-jasmine2 should work painlessly for the common cases - Please let me know if that's not the case.

Unfortunately, that doesn't seem to be the case according to https://github.com/facebook/jest/blob/master/packages/jest-circus and https://github.com/facebook/jest/blob/master/packages/jest-types/src/Circus.ts - jest-circus defines its own API, not compatible with Jasmine.
Also, I couldn't find a way to obtain expected/actual values for failed assertions in jest-circus API. Am I missing something? Any help would be much appreciated. @G-Rath @SimenB

---

#6295 (comment)

@G-Rath:

@segrey thanks for the quick & detailed response - I'm keen to get the gap between jest-circus & IntelliJ closed as fast as possible, so more than happy to help out wherever I can.

I'll post a kick-off comment on #8840 to try and scope out how much work it requires, and hopefully with a bit of guidance from @SimenB (or similar peoples that know the reporters side of jest in decent detail) I should be able to tackle at least some of it.

My understanding is that jest-circus is completely backwards compatible,

I can't find where I read that, and it was at least half a year ago, so I'm thinking I probably misunderstood the original posting.

Just to make sure I'm understanding:

Also, I couldn't find a way to obtain expected/actual values for failed assertions in jest-circus API. Am I missing something?

Is that the only primary thing that's missing for you right now, or is there more? (don't expect a full list, but just trying to scope out how big the gap actually is right now).

It'll probably be worth tracking this via a separate issue, depending on what @SimenB thinks. In the meantime I'll have a dig around jest-circus internals to see if I can load some more context into my head :)

---

#6295 (comment)

@segrey:

Is that the only primary thing that's missing for you right now, or is there more?

Also, I stumbled upon adding a custom Circus.EventHandler via addEventHandler: as I can see there is addEventHandler(environment.handleTestEvent.bind(environment)) in jest-circus/src/legacy-code-todo-rewrite/jestAdapterInit.ts, but IntelliJ cannot overwrite existing Jest environment.
Is there a way to add a custom Circus.EventHandler from a script registered via setupFilesAfterEnv?

[G-Rath]

@segrey I own you an apology: I originally thought that the js files used by IntelliJ were packed into the jar, but I see they're on disk.

Having those sources should hopefully make things a bit easier, as it means just comparing all the APIs that are used, and ensuring there are equivalents.

Unfortunately I can't actually edit them, which I am a bit surprised about - I didn't think an application could lock out the Windows superuser, but I don't feel a strong urge to brick my laptops permissions quite yet 😂

[G-Rath]

#6295 (comment)

No official way (without private imports from jest-circus or the likes) afaik. I've been critical of the inflexible test environment abstraction before :/

@jeysal How hard do you think it'd be to add that?

I mean, when you say "without private imports" does that mean it could be roughly a matter of just making those imports not private? (or maybe just documentation private?).

Even if that just gets us half the way that would be progress.

[SimenB]

As an IntelliJ user myself, I'm very keen to support integration with it as well as possible.

My understanding is that jest-circus is completely backwards compatible,

If you use the publicly documented features, it is. The Jasmine API (including the reporters) aren't documented, so that's not included in that compatibility promise.

Even if that just gets us half the way that would be progress.

It might also paint us into a corner though. One idea with circus is that people should be able to do import {test} from 'jest-circus', so adding more exports to it is not really ideal at this point.

---

I think an interesting approach to solving this issue might be to look into exposing all circus events on the reporter somehow (essentially funnel JestEnvironment.handleTestEvent into reporters). It should probably be preceded by making reporters listen to events instead of implementing an interface. I started a tiny bit in #6616 (comment) (@rogeliog took it a bit further in #9001, might be worth looking at as well). That got stuck on events from worker to parent, but that's orthogonal to rewriting reporters as event listeners (since in this case it's ok to wait until the test has completed running). Once we're sending events around, it's easier to add more events, such as events from jest-circus.

Any reason not to have this discussion in #8840 though?

[segrey]

Unfortunately I can't actually edit them, which I am a bit surprised about - I didn't think an application could lock out the Windows superuser, but I don't feel a strong urge to brick my laptops permissions quite yet 😂

Ah, a surprise indeed. At least you can download IDE as a .zip archive from https://www.jetbrains.com/idea/download/#section=windows and extract to a convenient location.

[segrey]

I was thinking that this issue targets direct consumption of jest-circus API by IntelliJ and #8840 is about improving Jest reporter API so it will be possible to get rid of additional reporters for jest-jasmine2 / jest-circus. In other words this issue is a short-term solution, while #8840 is a long-term.
Please feel free to correct me.

#9001 looks promising, cool.

that's orthogonal to rewriting reporters as event listeners

Do you mean to rewrite a Jest reporter as jest-circus event listener? Wouldn't it require more public jest-circus API?

[SimenB]

The orthogonal part I was referring to is reporting events from the worker to the parent process while it's running, instead of the single request-response flow that's there today. For our purposes here (a refactor of the current features of the reporter) that restraint doesn't matter.

Consuming jest-circus directly is probably a way bigger issue, I'm not sure where to even begin. We don't want to expose anything circus-like into the test environments themselves (like the jasmine global or how we inject the jest object) unless it's the only way. If we can get away with making reporters more powerful, I think that's easier both in terms of implementation in Jest and complexity for integrations, both medium and long-term

[segrey]

@SimenB Thanks for the quick reply and for the details! Totally agree with making reporters more powerful, that would be a relief for integrations!

[G-Rath]

@SimenB @segrey huge thanks for your detailed responses!

I've only just started to have the time to response due to a sudden explosion of work, but I plan to try and tackle at least some of #8840 over the weekend.

If you use the publicly documented features, it is. The Jasmine API (including the reporters) aren't documented

That makes complete sense - thanks for clarifying 👍

At least you can download IDE as a .zip archive from https://www.jetbrains.com/idea/download/#section=windows and extract to a convenient location.

Ah yes good point - I'll do that :)

Any reason not to have this discussion in #8840 though?
In other words this issue is a short-term solution, while #8840 is a long-term.

Sort of - I made this issue to avoid bogging down the original with something that isn't a direct blocker.

I consider this issue to be a sort of Epic or milestone, where the focus is on figuring out the answer to the question in the title: What is required for interop with IntelliJ? and following that, the tracking of the issues opened to in turn track the specific action points we come up with here :)

Having identified that #8840 is a chief milestone for achieving the goal set by this issue, I'm happy for the conversation related to that to be moved over there 🙂

[jeysal]

I've added this to the 26.x milestone because I believe that before circus becomes the default, it should integrate well with developer tooling, and that should have been proven in a Jest minor version.

[dandv]

Updating this thread as of the current WebStorm WAP (WebStorm 2021.1 EAP, Build #WS-211.5538.3, built on February 1, 2021)

editor-level highlighting of erroring matchers (i.e red squiggles & popup messages a la eslint & ts)

I do see those now:

A new issue I see is that while the test is still executing, all describes still show the spinner, even though the tests within have completed (and even passed):

This is a bigger problem for large test suites, because the user can't easily tell which tests are still executing.

Another issue is that WebStorm shows "Expected :undefined / Actual :undefined" on this code:

test('jest-circus - undefined not useful', () => {
  const obj = {
    a: [1, { foo: 'bar' }],
  };
  expect(obj).toMatchObject({
    a: [1],
  });
});

Those two lines don't show in the console output, or in WebStorm without jest-circus.

[G-Rath]

@SimenB @segrey I'm going to close this given we landed my PR which exposed the failure details combined with the YT issue on the IDEA side that implemented the inline preview support that the jasmine runner had, along with a number of other fixes & features that brought circus support roughly to the same level.

While I'm sure there will be further improvements to be made on both sides, it's probably going to be best to create new issues to track those.

---

@dandv I've checked your code example, and WebStorm no longer shows the "Expected/Actual" as being undefined:

Error: expect(received).toMatchObject(expected)

- Expected  - 0
+ Received  + 3

  Object {
    "a": Array [
      1,
+     Object {
+       "foo": "bar",
+     },
    ],
  }

I suspect the spinner problem you've mentioned is best reported on YouTrack, as it's likely an IDEA thing rather than jest-circus (I could be very wrong on that tho 🤷), but if it does belong on jests side I think best report it as a new issue :)

[github-actions]

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.