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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature Proposal][circus] Add assertion events to jest-circus or event custom events #10059

Closed
Bnaya opened this issue May 18, 2020 · 12 comments

Comments

@Bnaya
Copy link

Bnaya commented May 18, 2020

馃殌 Feature Proposal

Add assertion events to jest circus,
and expose api for custom assertion libraries to emit events as-well

Motivation

Jest circus events provide valuable info during test execution.
I believe that adding events for assertions will give even a better picture of what's going on during the event execution.
My personal motivation is that I'm writing a tool that extends jest to show additional information on test runs & during test runs, and to be able to also show successful assertions.
Without standard way from jest side, I would have to wrap the assertion library myself.

That can be helpful to various tools,
And maybe even solve things like:
#6295 (comment)

Example

I don't have something very solid in mind, but if we take a look at:

Very crud dump of jest circus events during test run:

setup
add_hook
start_describe_definition hey
add_test yo
add_test oops
finish_describe_definition hey
run_start
run_describe_start
run_describe_start
test_start yo
hook_start
hook_success yo
test_fn_start yo
test_fn_success yo
test_done yo
test_start oops
hook_start
hook_success oops
test_fn_start oops
test_fn_failure oops
test_done oops
run_describe_finish
run_describe_finish
run_finish
teardown

The events i'm thinking about are (They must be sync, the return promise api is not applicable here unfortunately):

assertion_start 
assertion_failure / assertion_success
assertion_end

With additional metadata that would make sense there

Pitch

I think the power of jest-circus is the best pitch :)
Exposing more information as events from the test runs and even custom events, will give more power to external tools without the need to to integrate with the core directly, monkey patch or wrap things etc

@benjamingr
Copy link
Contributor

@SimenB hey any chance this could happen? What are some next steps to promote such a feature request in Jest?

@benjamingr
Copy link
Contributor

Ping attempt #2 @SimenB

@jeysal
Copy link
Contributor

jeysal commented Jun 7, 2020

I'm a bit worried about coupling the test runner strongly to the assertion library here. There's only a tiny bit of code in circus that links together expect, jest-snapshot, etc. at the moment. Some sort of custom events API might make sense as it also allows for integrating other assertions libraries (isn't specific to expect) and even other completely unrelated use casess. It'd have to be carefully designed to not conflict with potential future changes to circus of course. Also, I'm not sure how much this helps on its own, since you'd also need custom bits in the test runner output (TestResult) to actually get info through to the reporter, right?

@Bnaya
Copy link
Author

Bnaya commented Jun 7, 2020

Thank you for your response!

From my hight level goal:

Any help from jest side so I won't need to patch things to get instrumentations would be super helpful (And not only for my use case :))

Or even if I will patch the assertion library myself, using circus events to get the data out would be nicer than using my own mechanism.

Examining circus code, I see how i can add my custom events by writing my own circus-based runner, or requiring dispatch from jest-circus/src/state.ts but that's abstraction level I think It's better not to cross, but to provider api for that.

I'm a bit worried about coupling the test runner strongly to the assertion library here
sort of custom events API might make sense

I totally agree, if that's sounds reasonable enough I can open more focused proposal

since you'd also need custom bits in the test runner output (TestResult) to actually get info through to the reporter, right?

For my use case I'm avoiding that part by writing to disk in a known location per test (by test name, and all)
That's far from perfect, but simplifies many things for us now.
If there were api also for that, that would also be nice :)

@jeysal
Copy link
Contributor

jeysal commented Jun 7, 2020

So for circus I could see custom event dispatching being accepted and merged, if in circus itself it's wrapped as a 'user event' inside a 'circus event' {name: 'custom', payload: {name: string, ... }} so that it cannot conflict with circus' own business but only intercepted by custom event handlers added to circus via the already existing API.

For the expect part of this (haven't touched on that in my previous message) I think patching or similar would be required, I don't think we want to make expect that extensible. That's just how you'd integrate any custom event into the test code, as most libraries used in tests (assertion or otherwise) won't provide you an event system to hook into on their own.
But I don't think it's a problem, in fact you don't really need to patch, you can build your own facade for expect instead and import that from somewhere, which is much cleaner. (Usage of that instead of standard global expect can also be enforced by a lint rule..)

@Bnaya
Copy link
Author

Bnaya commented Jun 7, 2020

Sorry for diverging from the main topic, but:
What do you think about adding api to add opaque data to reporting,
So custom reporters can read on the other side?
(In a separate proposal & PR)

Both the events api & report api may be exposed to the jest environment, or even the test itself

@benjamingr
Copy link
Contributor

But I don't think it's a problem, in fact you don't really need to patch, you can build your own facade for expect instead and import that from somewhere, which is much cleaner.

Would it be helpful to jump on a quick call and explain the use case we have for this?

We will (happily) allocate developer time to work on adding this (custom metadata) to circus but I think it also makes sense to add hooks into assertions.

There are multiple use cases for detecting successful assertions including but not limited to:

  • Collecting metadata on every successful assertion (for example page screenshot when testing browsers).
  • Verifying certain assertions have run (the classic example is failing a test that has no assertions).
  • Verifying assertions have run in the particular way (for example, that assertions always run after fake timers were ticked).

On the other hand, an implementation should not penalize users not using events in terms of performance or API - in bluebird we did this with config (called "monitoring").

@SimenB
Copy link
Member

SimenB commented Jun 8, 2020

While some sort of hook for adding custom events might make sense at some point, I think assertions are such an integral part of test runners that assertion events makes sense as a first-class feature of Circus. While I don't think we should couple Circus directly to expect, creating some sort of binding that allows expect to emit those events to circus which can then decorate and re-emit them sounds good to me. If you've used Cypress it lists out every single assertion made, and I think it's a valuable feature. I think this ties nicely into #6616 (although that's scoped to tests and not assertions) & #8840.

Not sure how to best make expect emit events and such. It already has a state (where it keeps number of assertions), but that quickly gets wonky with concurrent tests since expect is not bound to a specific test making it hard (if not impossible) to know which test is currently running. I think that would be an issue here as well. Concurrent tests is experimental and not documented though, so maybe not a huge deal? That state could possibly keep an event emitter instance (not that since it needs to work in the browser, but conceptually) or some such?

@Bnaya
Copy link
Author

Bnaya commented Jun 18, 2020

@SimenB What do you think the best way to move it forward?
We probably going to have time to work on it on the coming 2 weeks

I thinking about:

  • exposing custom events first in PR 1
    • should we force them to be serialisable?
  • Adding special custom event assertion event + expect integration in PR 2
    • As I see it, The main challenge here would be to reasonably translate the various kinds expect calls to a well-defined format, that other assertion libraries may also use.

Regarding adding opaque data to reporter api,
I've created a separate feature proposal:
#10175

@github-actions
Copy link

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 14 days.

@github-actions github-actions bot added the Stale label Feb 25, 2022
@github-actions
Copy link

This issue was closed because it has been stalled for 7 days with no activity. Please open a new issue if the issue is still relevant, linking to this one.

@github-actions
Copy link

github-actions bot commented May 5, 2022

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 May 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants