Skip to content

expect().resolves and expect().rejects#3318

Merged
Jarred-Sumner merged 9 commits intomainfrom
test-each
Jun 23, 2023
Merged

expect().resolves and expect().rejects#3318
Jarred-Sumner merged 9 commits intomainfrom
test-each

Conversation

@Electroid
Copy link
Copy Markdown
Contributor

@Electroid Electroid commented Jun 14, 2023

Adds support for:

import { test } from "bun:test";

test("resolves and rejects", () => {
  expect(Promise.resolve()).resolves.toBeUndefined();
  expect(Promise.reject(new Error()).rejects.toBeInstanceOf(Error);
});

@Electroid Electroid added enhancement New feature or request bun:test Something related to the `bun test` runner labels Jun 14, 2023
@Electroid Electroid requested a review from Jarred-Sumner June 14, 2023 23:25
@Electroid
Copy link
Copy Markdown
Contributor Author

Also, you'll want to review this PR per-commit, since the first commit is just moving code from 1 file to several.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 14, 2023

@Jarred-Sumner 10 files with test failures on bun-darwin-aarch64:

  • test/bundler/bundler_browser.test.ts
  • test/bundler/esbuild/default.test.ts
  • test/bundler/esbuild/importstar.test.ts
  • test/cli/install/bunx.test.ts
  • test/js/bun/eventsource/eventsource.test.ts
  • test/js/bun/test/test-test.test.ts
  • test/js/bun/websocket/websocket-server.test.ts
  • test/js/third_party/socket.io/socket.io-close.test.ts
  • test/js/web/fetch/fetch-leak.test.js
  • test/js/web/websocket/websocket.test.js

View test output

#28546b1c03ad8c09cd0087ad4dc3308b61c04755

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 14, 2023

@Jarred-Sumner 8 files with test failures on linux-x64-baseline:

  • test/bundler/bundler_browser.test.ts
  • test/bundler/esbuild/default.test.ts
  • test/bundler/esbuild/importstar.test.ts
  • test/cli/install/bunx.test.ts
  • test/js/bun/util/inspect.test.js
  • test/js/bun/websocket/websocket-server.test.ts
  • test/js/third_party/prisma/prisma.test.ts
  • test/js/web/websocket/websocket.test.js

View test output

#28546b1c03ad8c09cd0087ad4dc3308b61c04755

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 14, 2023

@Jarred-Sumner 9 files with test failures on linux-x64:

  • test/bundler/bundler_browser.test.ts
  • test/bundler/esbuild/default.test.ts
  • test/bundler/esbuild/importstar.test.ts
  • test/cli/install/bunx.test.ts
  • test/js/bun/util/inspect.test.js
  • test/js/bun/websocket/websocket-server.test.ts
  • test/js/third_party/prisma/prisma.test.ts
  • test/js/third_party/socket.io/socket.io-close.test.ts
  • test/js/web/websocket/websocket.test.js

View test output

#28546b1c03ad8c09cd0087ad4dc3308b61c04755

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 15, 2023

@Jarred-Sumner 10 files with test failures on bun-darwin-x64-baseline:

  • test/bundler/bundler_browser.test.ts
  • test/bundler/esbuild/default.test.ts
  • test/bundler/esbuild/importstar.test.ts
  • test/cli/install/bunx.test.ts
  • test/cli/test/bun-test.test.ts
  • test/js/bun/spawn/spawn-streaming-stdin.test.ts
  • test/js/bun/sqlite/sqlite.test.js
  • test/js/bun/websocket/websocket-server.test.ts
  • test/js/web/timers/setTimeout.test.js
  • test/js/web/websocket/websocket.test.js

View test output

#28546b1c03ad8c09cd0087ad4dc3308b61c04755

@Jarred-Sumner
Copy link
Copy Markdown
Collaborator

@Electroid we want mark the Promise as handled. Promises internally hold a state flag. Promise.reject() goes into the rejection queue immediately, but then at the end of the next tick, when promise rejections are drained, we check if the rejected Promise was handled. If handled, it is not reported.

You'll need to read JSC's code to figure out exactly where, but it will ultimately be an |= on this flag:

promise->internalField(JSC::JSPromise::Field::Flags).set(arg0->vm(), promise, jsNumber(static_cast<unsigned>(JSC::JSPromise::Status::Fulfilled)));

Comment thread src/bun.js/test/expect.zig Outdated
};
value.ensureStillAlive();

if (this.resolves) |resolves| {
Copy link
Copy Markdown
Collaborator

@Jarred-Sumner Jarred-Sumner Jun 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main question here is if value will need to be wrapped in a Promise as well.

The pattern is

await expect(fetch(url)).resolves.toBeInstanceOf(Response)

Sometimes, people do this:

try {
   expect(123).toBe(456);
} catch (e) {

} 

So if they do that with a Promise and call .then, .catch, .finally etc, their code will throw an error.

@Jarred-Sumner Jarred-Sumner merged commit 217501e into main Jun 23, 2023
@Jarred-Sumner Jarred-Sumner deleted the test-each branch June 23, 2023 05:27
Jarred-Sumner added a commit that referenced this pull request Jun 24, 2023
* Move expect and snapshots to their own files

* expect().resolves and expect().rejects

* Fix promise being added to unhandled rejection list

* Handle timeouts in expect(<promise>)

* wip merge

* Fix merge issue

---------

Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
Co-authored-by: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com>
Jarred-Sumner added a commit that referenced this pull request Jun 24, 2023
* wip changes for CommonJS

* this rewrite is almost complete

* even more code

* wip

* Remove usages of `import.meta.require` from builtins

* Remove usages of require

* Regenerate

* ✂️ builtin rewrite commonjs in printer

* Use lazy custom getters for import.meta

* fixups

* Remove depd

* ugh

* still crashing

* fixup undici

* comment out import.meta.require.resolve temporarily

not a real solution but it stops the crashes

* Redo import.meta.primordials

* Builtins now have a `builtin://` protocol in source origin

* Seems to work?

* Finsih getting rid of primordials

* switcharoo

* No more function

* just one more bug

* Update launch.json

* Implement `require.main`

* ✂️

* Bump WebKit

* Fixup import cycles

* Fixup improt cycles

* export more things

* Implement `createCommonJSModule` builtin

* More exports

* regenerate

* i broke some stuff

* some of these tests work now

* We lost the encoding

* Sort of fix zlib

* Sort of fix util

* Update events.js

* bump

* bump

* bump

* Fix missing export in fs

* fix some bugs with builtin esm modules (stream, worker_threads, events). its not perfect yet.

* fix some other internal module bugs

* oops

* fix some extra require default stuff

* uncomment this file but it crsahes on my machine

* tidy code here

* fixup tls exports

* make simdutf happier

* Add hasPrefix binding

* Add test for `require.main`

* Fix CommonJS evaluation order race condition

* Make node:http load faster

* Add missing exports to tls.js

* Use the getter

* Regenerate builtins

* Fix assertion failure in Bun.write()

* revamp dotEnv parser (#3347)

- fixes `strings.indexOfAny()`
- fixes OOB array access

fixes #411
fixes #2823
fixes #3042

* fix tests for `expect()` (#3384)

- extend test job time-out for `darwin-aarch64`

* `expect().resolves` and `expect().rejects` (#3318)

* Move expect and snapshots to their own files

* expect().resolves and expect().rejects

* Fix promise being added to unhandled rejection list

* Handle timeouts in expect(<promise>)

* wip merge

* Fix merge issue

---------

Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
Co-authored-by: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com>

* fixup min/memcopy (#3388)

* Fix crash in builtins

* Don't attempt to evaluate modules with no source code

* Update WebCoreJSBuiltins.cpp

* Update WebCoreJSBuiltins.cpp

* Update WebCoreJSBuiltins.cpp

* Fix crash

* cleanup

* Fix test

cc @paperdave

* Fixup Undici

* Fix issue in node:http

* Create util-deprecate.mjs

* Fix several bugs

* Use the identifier

* Support error.code in `util.deprecate`

* make the CJs loader slightly more resilient

* Update WebCoreJSBuiltins.cpp

* Fix macros

---------

Co-authored-by: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com>
Co-authored-by: dave caruso <me@paperdave.net>
Co-authored-by: Alex Lam S.L <alexlamsl@gmail.com>
Co-authored-by: Ashcon Partovi <ashcon@partovi.net>
Co-authored-by: Ciro Spaciari <ciro.spaciari@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bun:test Something related to the `bun test` runner enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants