Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bun:test promise rejection resolves instead of rejects when throw after await with done callback #1546

Open
Jarred-Sumner opened this issue Nov 23, 2022 · 8 comments
Labels
bug Something isn't working jest Something related to the `bun test` runner

Comments

@Jarred-Sumner
Copy link
Collaborator

In the following test, if expect().toBe() fails then the test hangs forever

it("should allow stdout to be read via .read() API", async (done) => {
    const result: string = await new Promise((resolve) => {
      const child = spawn("bun", ["-v"]);
      let finalData = "";
      child.stdout.on("error", (e) => {
        console.error(e);
        done(e);
      });
      child.stdout.on("readable", () => {
        let data;

        while ((data = child.stdout.read()) !== null) {
          finalData += data.toString();
        }
        resolve(finalData);
      });
    });
    expect(SEMVER_REGEX.test(result.trim())).toBe(true);
    done();
});

The reason why is that the Promise handler for test appears to be calling resolve instead of reject.

@Jarred-Sumner Jarred-Sumner changed the title bun:test promise rejection resolves instead of rejects when throw after await with with done callback bun:test promise rejection resolves instead of rejects when throw after await with done callback Nov 23, 2022
@Electroid Electroid added jest Something related to the `bun test` runner bug Something isn't working labels Nov 23, 2022
@mmkal
Copy link

mmkal commented Jul 21, 2023

I don't know if this is the same bug, but .rejects doesn't seem to work at all. Here's a simpler repro - test passes in jest/playwright but fails in bun:

const action = async (): Promise<unknown> => {
  return Promise.reject(new Error('some error'))
}

test('reject', async () => {
  await expect(action()).rejects.toThrow('some error')
})

@Jarred-Sumner
Copy link
Collaborator Author

As a temporary workaround, can you try putting action() in a function?

const action = async (): Promise<unknown> => {
  return Promise.reject(new Error('some error'))
}

test('reject', async () => {
  await expect(() => action()).rejects.toThrow('some error')
})

@mmkal
Copy link

mmkal commented Jul 21, 2023

Looks like that works, yes - and in playwright, jest and deno too. Thanks!

@tlgreg
Copy link

tlgreg commented Aug 14, 2023

The last example doesn't seem to be working for me. (bun 0.7.3)
Seems like an issue with the combination of .rejects and .toThrow actually.
If the received is not a function toThrow breaks on that and if it is then rejects fails it on a mismatch.

const action = () => Promise.reject('some error')

test('Promise to be', async () => {
  await expect(Promise.reject('some error')).rejects.toBe('some error')
}) // passes

test('Promise to throw', async () => {
  await expect(action()).rejects.toThrow('some error')
}) // error: Expected value must be a function

test('=> Promise to throw', async () => {
  await expect(action).rejects.toThrow('some error')
}) // error: Expected promise, Received: [Function]

@franciscop
Copy link

I just found this after some searching, @tlgreg the current workaround I found is really non-compliant and I documented it in this issue (@Jarred-Sumner feel free to mark as duplicated since I just found this issue):

test("rejects to octopus", () => {
  // This should not be receiving a callback, but it's the only way it works:
  const rejecting = Promise.reject(() => new Error("octopus"));
  return expect(rejecting).rejects.toThrow("octopus");
});

@franciscop
Copy link

franciscop commented Sep 10, 2023

As a further workaround, this way it can be compatible with both Node.js and Bun (in my case it's a single test that is failing so I've done this until it's fixed upstream):

test("rejects to octopus", () => {
  let rejecting;
  if (typeof Bun !== "undefined") {
    rejecting = Promise.reject(() => new Error("octopus"));
  } else {
    rejecting = Promise.reject(new Error("octopus"));
  }
  return expect(rejecting).rejects.toThrow("octopus");
});

Note another non-compliance bit is that if that first line was let rejecting = Promise.reject(new Error(...)); instead of the else, the test fails, while it should not be failing (a rejected promise that is not awaited for should not be throwing/failing a test).

@lukechilds
Copy link

Also hitting this issue and it's blocking our migration since we don't have control of the rejection logic.

@franciscop
Copy link

This might be resolved since the related issue is resolved as well:

#4755 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working jest Something related to the `bun test` runner
Projects
None yet
Development

No branches or pull requests

6 participants