avajs / ava

Node.js test runner that lets you develop with confidence 🚀
MIT License
20.74k stars 1.41k forks source link

Support assertions during teardown #2613

Closed Silic0nS0ldier closed 3 years ago

Silic0nS0ldier commented 3 years ago

Technically this can be achieved now via teardown functions, however output indicates that is not a supported behaviour via an error message like the following;

Error: Assertion passed, but test has already finished

An example use case for this is asserting that a logging interface receives the expected output (specifically, that it has been called the expected number of times).

novemberborn commented 3 years ago

The teardown functions run at a point where we've already decided the test outcome.

An example use case for this is asserting that a logging interface receives the expected output (specifically, that it has been called the expected number of times).

That should be a part of the test itself. Why do you want to do this in a teardown? Perhaps you could wrap your test function to add the assertions?

Alternatively there is a test.afterEach() hook which runs for passed tests.

(I'm closing this issue for housekeeping purposes, but let's keep the conversation going.)

Silic0nS0ldier commented 3 years ago

While it could be a part of the test itself, additional work would be required to ensure the check if performed when trying to reuse code (or alternatively a rather unintuitive callback approach could be used). What I tried (which I prefer) resembles the following.

import test from "ava";
import { createTestAssertLogger, LogLevel } from "../log-test-assert.js";
test("Foo bar", t => {
    const log = createTestAssertLogger(t, [
        [ LogLevel.INFO, "foobar" ]
    ]);
    someFunction(log, ...);
});

With this approach the assertions are (relatively) cleanly defined, and there is the implication that the test completing with remaining log assertions would trigger a failure. A manual callback (ESLint providing a reminder) could also work, but I'm of the mindset that the extra boilerplate distracts from test composition.

test.afterEach() does technically offer a way to address this, but at the expense of a more complex setup (and more boilerplate, subject to how it is implemented). test.afterEach() also implies that this is something I want to do for every single test, which isn't necessarily true.

Given t.teardown() is called after the test outcome is determined, perhaps a different hook that is called before would be a better fit. In hindsight, it would be good to keep such activities separate. Avoids the issue of trying to perform assertions on resources that have already been destroyed.

novemberborn commented 3 years ago

There's also t.try() but that doesn't really do it either.

The work I started in https://github.com/avajs/ava/pull/2435 would let you create custom test() functions, and this could be one of those customizations. So you'd construct a test() which already has an afterEach to run additional assertions. You should keep an eye on that (but no promises as to when there'll be any progress, sorry).

Silic0nS0ldier commented 3 years ago

As you noted, t.try() isn't quite what I'm looking for (and using it in such a way feels like an anti-pattern given its documented scope).

Your mention of a custom test() function got me thinking. It should be possible to wrap the test function, such that additional functionality can be injected into the execution context (t), namely an in-test t.after() hook.

novemberborn commented 3 years ago

Your mention of a custom test() function got me thinking. It should be possible to wrap the test function, such that additional functionality can be injected into the execution context (t), namely an in-test t.after() hook.

Yea.

I have a setUp() helper which returns test and serial functions that have all the right hooks already registered. It works well enough.