Proposal: Pass expect as a param in bdd tests

[WWRS]

Is your feature request related to a problem? Please describe.

Leads to natural solutions to

• expect.hasAssertion and expect.assertions silently pass outside bdd tests #6518: expect outside @std/testing tests will not have the methods that are failing
• expect.hasAssertion and expect.assertions pollute other assertions tests #6540: Replacing the global AssertionState with one passed in the context guarantees assertions are counted in the right context, and possibly in parent contexts

Describe the solution you'd like

• Create a new interface ItTestContext extends Deno.TestContext which can include an AssertionState
  • Make it and test use ItTestContext instead of Deno.TestContext
• Remove @std/expect's expect.hasAssertion() and expect.assertions() to prevent them from being used outside bdd tests
• Add a way to fetch an instance of expect from a bdd test
  • This instance does support expect.hasAssertion() and expect.assertions()

Options for syntax:

import { makeExpect } from "@std/testing/expect";
test("test1", (t) => {
  const expect = makeExpect(t);
  expect.assertions(0);
});

test("test1", (t) => {
  t.expect.assertions(0);
  // This could have some issues where a user uses the wrong expect by accident,
  // for example this assertion would not be counted:
  // import { expect } from "@std/expect";
  // expect(1).toBe(1);
});
test("test2", (t) => {
  const { expect } = t;
  expect.assertions(0);
});

test("test1", (_t, expect) => {
  expect.assertions(0);
});

test("test1", (t) => {
  // No expect at all, just put the assertions tests into the context
  t.assertions(0);
});

Describe alternatives you've considered

I don't see a way to improve #6518 without moving AssertionState to some sort of context: The alternative is to add some sort of way for expect to know whether it's being called in a bdd test. If not, it would need to avoid counting assertions, and throw in the assertions test functions.

There might be a way for #6518 to be fixed by having it track whether it's been cleaned up and throw if not. The cleanup would happen in bdd tests but not in raw Deno.tests.

For #6540, the direct issue can be solved by only throwing the error after cleanup. However, I think it's still unintuitive that the same AssertionState is used for multiple tests. Even Jest has this issue:

describe('a', () => {
  // Fails with "expected 2, found 1", which does not make sense:
  // If we were counting only top-level, this should find 0
  // If we are counting in all children, this should find 2
  expect.assertions(2);

  test("1", () => {
    expect(1).toEqual(1);
  });

  test("2", () => {
    expect(1).toEqual(1);
  });
});

More extreme alternatives could involve preventing expect from running outside bdd tests. This would prevent users from using different sources of expect, but would also cause expect to be unavailable outside Deno.test() which is probably not a good idea.

[WWRS]

Of the options, I like 3 the best. It seems least intrusive.

I can implement this if approved.

[stefnotch]

Would it be unreasonable to support expect.hasAssertion in normal tests?

As in

Deno.test("foo", async (t) => {
  expect.assertions(1);
  let value = await computeBar();
  expect(value).toBe(1);
});

If that should be supported, then I believe that const expect = makeExpect(t); is the only viable API choice. Even if it's clearly less nice than option 3.

[WWRS]

Currently expect.hasAssertion is not supported in Deno.test but there's no indication of this (#6518). The specification of that function requires it be checked at the end of the test, and the current implementation only checks at the end of bdd tests. I would have to look into whether that can be ported to Deno.test.

Regardless, I think it makes more sense to have that sort of info in the TestContext than in a global, the latter being how it's currently implemented.

[stefnotch]

Sounds good, and do feel free to ping me whenever you'd like a review or a second opinion.