Proposal: Pass `expect` as a param in bdd tests
- Dominant language
- TypeScript
- Stars
- 3.6k
- Forks
- 681
- PR merge metrics
- No merged PRs in 30d
Description
**Is your feature request related to a problem? Please describe.**
Leads to natural solutions to
- #6518: `expect` outside `@std/testing` tests will not have the methods that are failing
- #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:
```ts
import { makeExpect } from "@std/testing/expect";
test("test1", (t) => {
const expect = makeExpect(t);
expect.assertions(0);
});
```
```ts
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);
});
```
```ts
test("test1", (_t, expect) => {
expect.assertions(0);
});
```
```ts
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.test`s.
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:
```ts
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.
Contributor guide
Assessment
This issue has not been assessed yet.