no multiple done()s and writing custom rules

[gaz7]

Some of our developers write tests with multiple done()s in them. This obviously isn't correct, as tests should run (and complete) the same way every time. An example might be:

myObservable$.subscribe({
  next: (result) => {
    expect(result).toBe('foo');
    done();
  },
  error: (err) => {
    done(): // this is wrong. it should fail the test if error happens
  }
});

Is there an existing rule that can catch this?

Whether this rule exists or not, I've been considering writing some custom lint rules for our codebase to handle our specific best practices.
Can we leverage some of the existing code/utilities in this lib to do that? If so, are there any guidelines?

Thanks in advance

[G-Rath]

Is there an existing rule that can catch this?

No there isn't - we actually recommend using promises instead of done, though I can't decide if that would help in this case?

From a detection point of view, it would be easier to report on "done called multiple times", but with a promise you might find people naturally write code that works e.g.

it('works', async () => {
  const { promise, resolve, reject } = Promise.withResolvers();

  myObservable$.subscribe({
    next: (result) => {
      expect(result).toBe('foo');
      resolve();
    },
    error: (err) => {
      reject();
    }
  });

  return promise;
});

while technically it is valid to call resolve in the error path, I feel like most people would default to using reject, and reviewers would notice if you were resolveing?

Can we leverage some of the existing code/utilities in this lib to do that? If so, are there any guidelines?

Our utilities live here and are technically accessible via the package but they're not officially part of the public API and we don't include types for them - though they've not changed in a while.

They're very self-contained so it should be very straightforward to just copy and paste what you need (which is also what other plugins like the Vitest one has done - there have been talks in the past about publishing these in a dedicated package, which I'm open to but so far no one has made a big enough case for that 🤷)

They're hopefully sufficiently named, implemented, and commented enough to make it easy to use them, though I'm happy to answer questions too.

The general idea is that when possible you start with parseJestFn, which takes a call expression, and do everything off that - the parsing handles determining if its a Jest function based on globals and imports, and resolves the chain to make it easy to do queries. Its test suite is pretty extensive too