Repository navigation
Planned assertion count / Hook into timeout? (for testing runtime) #44125
Description
Activity
- changed the title
[-]Planned assertion count / Hook into timeout?[/-][+]Planned assertion count / Hook into timeout? (for testing runtime)[/+]on Aug 3, 2022 Just to add a more real world example if it helps.
In the below example, the test will wait if
chatbotnever emit's amessageevent.test('test 2 assertions will timeout', { timeout: 2000 }, async t => { const assert = assertPlan(1); chatbot.addEventListener('message', (message) => { assert.strictEqual(message.content, 'hello world'); }) await assert.wait(); });
But all you'll get in the logs are:
TAP version 13 # Subtest: test not ok 1 - test --- duration_ms: 2.004530137 failureType: 'testTimeoutFailure' error: 'test timed out after 2000ms' code: 'ERR_TEST_FAILURE'But really, it would be helpful for me to somehow say:
assertPlan expected 1 assertion, but received 0 by the time the test timed out.I guess a solution could be the ensure we never use the test runners timeout, and rely soley on
assert-plan's timeout.It feels wrong to me, but I think I'd be easily convinced otherwise.
Thanks for the detailed issue. Just a few observations:
- If you're worried about an assertion not being run, in my experience, it's because some callback/handler function was never called. Check out
assert.CallTracker(). This is how Node's own test suite handles the problem. I'd say it's also less maintenance burden thanplan()which needs to be updated when assertions are added/removed. - Node's assertion library throws immediately on a failed assertion, so the plan will be wrong on failing tests. For passing tests, see my previous point on an alternate way to handle that.
- If we implemented
plan()we'd also probably need to wrap theassertmodule so that failed assertions don't throw. That would also make it more difficult to bring your own assertion library, and so we'd probably need a generic API to hook into arbitrary assertion libraries. - I don't think we should expose the timer that the test uses internally, since user code could tamper with it.
before(),beforeEach(),after(), andafterEach()hooks landed recently, but haven't been released yet. You might be able to leverage those as well.
Reacted by Moshe Atlow and Benjamin Gruenbaum- If you're worried about an assertion not being run, in my experience, it's because some callback/handler function was never called. Check out
We expose the AbortSignal in the test which has the event you describe and you can use.
On the other hand I do see value in exposing common.mustCall/common.mustNotCall we use extensively in our own tests maybe?
Reacted by Moshe Atlow and Antoine du HamelOn the other hand I do see value in exposing common.mustCall/common.mustNotCall we use extensively in our own tests maybe?
that is already possible - just combinie
assert.CallTracker()withprocess.on('exit').
that combination is actually the first example on the very beginning ofCallTrackerdocsReacted by Antoine du Hamel and Benjamin Gruenbaumalso, implementing a plan for testing (any plan, not just for assersions) can be done with a counter and checking it inside a
afterhookcan this issue be closed?
Thank you everyone for taking the time to help me and come up with so many ideas/suggestions. I really appreciate it.
The before/after hook seems like a great way to solve this. I'll try it once it get's published.
Reacted by Moshe Atlow and Benjamin GruenbaumHi folks, apologies for resurrecting an old issue but we find ourselves needing the assestion planning feature that exists in most testing framework such as tap and jest, and which is hard to replicate without native support from the test runner.
Here's a simple example of something you'd write in tap:
test('plan', t => { t.plan(1) doSomething(() => t.ok(true)) })
Rewriting this test without native support for assertion planing is tricky. One way would be to create a promise:
test('no plan', t => { let resolve const p = new Promise(_resolve => { resolve = _resolve }) doSomething(() => { t.ok(true) resolve() }) return p })
Clearly, this gets complicated and unfeasible very quickly, e.g. imagine nested callbacks, and has no easy user-space solution.
As much as I dislike test planning, because it makes tests harded to write and change, it's the only sensible way that I'm aware of to support assertions in callbacks, which are in some cases unavoidable.
Thoughts?
I have a branch implementing this exact thing, but was on the fence as to whether or not to PR it. It would be good to hear what others think since a fair amount of time has passed since this issue was originally opened.
I have a branch implementing this exact thing, but was on the fence as to whether or not to PR it. It would be good to hear what others think since a fair amount of time has passed since this issue was originally opened.
That's a good start and good to hear, thanks @cjihrig. I assume that this also includes wrapping the assert module so it plays well along with node:test, right?
Personally, I'd say that 90% of the test I come across don't need this, but when you do, not having this feature is a show stopper.
I assume that this also includes wrapping the assert module so it plays well along with node:test
That's correct. Here is some code from my branch:
test('plan passes with callbacks', (t, done) => { t.plan(3); t.assert.ok(true); setImmediate(() => { t.assert.ok(true); done(); }); t.assert.ok(true); });
And, I haven't implemented it yet, but I designed this such that in the future we could support:
- Using a different assertion library / adding additional assertion functions.
- Silent assertions (similar to how they work in tap).
That's the way I would imagine it would look like. Now if it also worked, it would be amazing ;)
@simoneb have a look at https://github.com/mcollina/tspl
/CC @mcollina@simoneb have a look at https://github.com/mcollina/tspl /CC @mcollina
Thanks, I wasn't aware that Matteo built one. I'm sure there are alternatives, it would be great to pick one and bundle it in node:test.
This is all it really takes to build it in: cjihrig@89d9a7e. Unlike a userland solution, this can also be easily extended to address #52033 (silent node-tap style assertions).
The fundamental reason I needed tspl was supporting Node 18 too.
Reacted by Simone Busoli
What is the problem this feature will solve?
When writing tests, I really like the
planfunction in tape.In summary it lets you specify how many assertions you expect to have, then the test will fail if there are fewer/more than planned.
I'm really loving the new built in testing runtime, but I'm really missing
plan. So I've built a wrapper around assert that implements the plan function in a similar way.https://github.com/markwylde/assert-plan#example-usage
It's working really well, but there's a problem when the test timesout. I would like to log out the test timed out due to not receiving enough planned assertions.
To get around this I have to provide two timeouts, as below: (it works, but it's not very nice):
For example
Notes:
assertPlantimes out, it throws and error that is outputted to the user.What is the feature you are proposing to solve the problem?
I think there's probably an obvious existing solution, or I'm trying to solve something in a strange way. So I'm really open to criticism on my approach. But some ideas off the top of my head are:
Proposal 1 - expose the timeout on
tProposal 2 - expose an event when a test timesout
Proposal 3 - test timeout hook
What alternatives have you considered?
I originally thought if we could add a
planfunction to the test runtime, liketapedoes:For example:
But it breaks the cleanest of the
testmodule, and how it's not coupled withassertat all. So I feel the right solution is to wrap/use an alternative assert library.Maybe we could add a
planto theassertmodule, but you could be running multiple tests in parallel andassertwouldn't have a way of accumulating counts for just the specific test you are running.You could make
asserta constructor, but even then the problem of how to timeout the wait when the test timeouts still exists.Thank you for humoring my idea and sorry if there is some really obvious stuff I'm missing here.