feat(bun:test) add support for test.each() and describe.each() #4047

jecquas · 2023-08-07T20:36:50Z

What does this PR do?

add support for test.each() and describe.each()

Documentation or TypeScript types (it's okay to leave the rest blank in this case)
Code changes

How did you verify your code works?

I wrote automated tests and ran bun test

I checked the lifetime of memory allocated to verify it's (1) freed and (2) only freed when it should be
I or my editor ran zig fmt on the changed files
I included a test for the new code, or an existing test covers it
JSValue used outside outside of the stack is either wrapped in a JSC.Strong or is JSValueProtect'ed
I added TypeScript types for the new methods, getters, or setters

Jarred-Sumner · 2023-08-08T01:53:51Z

src/bun.js/test/jest.zig

-    callback: JSC.JSValue,
+
+    func: JSC.JSValue,
+    func_arg: []JSC.JSValue,


in zig, slices store a len so we don't need to store both arg_size and len

Jarred-Sumner · 2023-08-08T01:54:59Z

src/bun.js/test/jest.zig

-            callback.unprotect();
-            this.callback = .zero;
+            var idx: u32 = 0;
+            while (idx < this.func_arg_size) {


I think you can do

Suggested change

while (idx < this.func_arg_size) {

for (this.func_args) |arg| {

arg.unprotect();

}

Jarred-Sumner · 2023-08-08T01:56:40Z

src/bun.js/test/jest.zig

+
+            if (item_is_array) {
+                // Spread array as args
+                const item_length = item.getLength(globalThis);


This should use JSC.JSArrayIterator to iterate through the array.

Jarred-Sumner · 2023-08-08T01:57:35Z

src/bun.js/test/jest.zig

+                // Spread array as args
+                const item_length = item.getLength(globalThis);
+                while (idx < item_length) : (idx += 1) {
+                    var arg = item.getIndex(globalThis, idx);


Every call to getIndex can potentially throw. If arg == .zero then it threw. If we do not handle exceptions, Bun will segfault.

Also, use const instead of var whenever possible

Jarred-Sumner · 2023-08-08T01:59:42Z

src/bun.js/test/jest.zig

+
+        while (iter.next()) |item| {
+            // TODO: node:util.format() the label
+            const label = if (description == .zero)


description will not be zero in this case, it would be .undefined

src/bun.js/test/jest.zig

Jarred-Sumner · 2023-08-08T02:42:50Z

Thank you for this, it's a good start

jecquas · 2023-08-08T18:39:56Z

changes made
still get a sigsegv when running all the tests, but happens on main as well (#4062)

Jarred-Sumner · 2023-08-09T02:01:32Z

src/bun.js/test/jest.zig

+
+pub const EachData = struct { strong: JSC.Strong, is_test: bool };
+
+fn eachBind(


This should really be two different functions or a single function with a comptime argument and then is_test becomes unnecessary

but that shouldn't block this from shipping

Jarred-Sumner · 2023-08-09T02:04:15Z

src/bun.js/test/jest.zig

+                var item_iter = item.arrayIterator(globalThis);
+                while (item_iter.next()) |array_item| {
+                    if (array_item == .zero) {
+                        allocator.free(function_args);


this leaks memory because existing function args will not be unprotected

but lets handle that a little later, this is very much an edgecase as-is

Jarred-Sumner · 2023-08-09T05:26:05Z

Thank you for this. Looks good but we should add the formatters too. I think a simplified version would be fine. Probably using the console.log formatter to handle %s, and other common ones.

…sh#4047) * rename callback to func * update testscope to handle function arguments * works * big cleanup * works in debug, not release * fix memory issue & update tests * catch & str test * write types for each() & switch tests to ts * rm & typo * move some code around & support describe * review changes

* rename callback to func * update testscope to handle function arguments * works * big cleanup * works in debug, not release * fix memory issue & update tests * catch & str test * write types for each() & switch tests to ts * rm & typo * move some code around & support describe * review changes

* Let the debugger to pause/resume the event loop * Add initial support for Debug Adapter Protocol * Add progress * Update Worker.cpp * Fix require("console") #3820 (#4073) * Fix #3820 * Add Module (#4074) * Set exports to {} in user-constructed CommonJSModuleRecords (#4076) * feat(bun/test): Implement "toSatisfy" & "toIncludeRepeated" (fwup) (#3651) * Fix merge issues * oop * make codegen * Fix build issues --------- Co-authored-by: dave caruso <me@paperdave.net> * worker tests (#4058) Co-authored-by: Jarred Sumner <jarred@jarredsumner.com> * feat(bun:test) add support for test.each() and describe.each() (#4047) * rename callback to func * update testscope to handle function arguments * works * big cleanup * works in debug, not release * fix memory issue & update tests * catch & str test * write types for each() & switch tests to ts * rm & typo * move some code around & support describe * review changes * Fix one of the astro segfaults, also fix bun init version (#4079) * 4->16 * add assertions * fix version stuff * Remove unintentional logs from #4043 * Run prettier * Update main-worker-file.js * Update SIMDUTF (#4078) * Fix constructing buffer from a UTF16 string with the Latin1 encoding. (#4086) Close: #3914 * Add support for `bun --revision` (#4027) Co-authored-by: Yash Sharma <yashsharma@Yashs-MacBook-Air.local> * Updates * Update __global.zig * remove non-node node-fallbacks (#4082) * remove non-node node-fallbacks. * organize the imports * Fix test * Sync bun-polyfills branch (#4081) * bun-polyfills: initial impl. & baseline refactor * move @types/ws dep from root to /test/ * bun-types: remove ReadableStream.forEach method (this does not exist, probably added by mistake) * bun-polyfills: remove extraneous stream utils * bun-polyfills: add types syncing file * bun-polyfills: re-arrange global polyfills * bun-polyfills: fix FileBlob streams types again * bun-polyfills: sync all of @types/node * bun-polyfills: typeguard all current polyfills * bun-polyfills: fix import paths * bun-polyfills: switch to wasm impl. of farmhash * bun-polyfills: support default import of bun obj * bun-polyfills: transpiler placeholder file * bun-polyfills: loaderless import.meta polyfill * bun-polyfills: refactor import.meta polyfill * bun-polyfills: repl entrypoint & todo list index * bun-types: Add null to return type of Bun.which * bun-types: match Bun.sha with Bun.hash.SHA512_256 * bun-polyfills: new "repl" package.json script * bun-polyfills: full refactor of toplevel hashes * bun-polyfills: these are fixed * bun-types: NODE_ENV is optional * bun-polyfills: fix Bun.env types * bun-types+polyfills: fix HeapSnapshot.version type * bun-polyfills: fix some web streams type conflicts * bun-polyfills: update internal FileBlob.slice * bun-polyfills: fix subproc stdin conversions * bun-polyfills: better internal fileblob types * bun-polyfills: try to sync global performance type * bun-polyfills: working zig wasm polyfills setup * bun-polyfills: update scripts * bun-polyfills: fix wasm file location resolution * bun-polyfills: goodbye farmhash (replaced by zig) * bun-polyfills: move all Bun.hash polyfills to zig * bun-polyfills: reimpl. seeding of seeded hashes * bun-polyfills: impl. undocumented murmur32v2 * bun-polyfills: switch zighash from jsdoc to .d.ts * bun-types: partial fix of Hash types * bun-polyfills: documented Hash.murmur32v2 * bun-polyfills: misc updates * bun-polyfills: enable sourcemaps * bun-polyfills: handle empty inputs to hash funcs * bun-types: narrow down hash func types * bun-polyfills: remove unnecessary bigint casts * bun-polyfills: impl. Bun.isMainThread * bun-polyfills: impl. Bun.sleep and fix sleepSync * bun-polyfills: impl. indexOfLine * bun-polyfills: impl. Bun.peek.status * bun-types: fix hashing test --------- Co-authored-by: Jarred Sumner <jarred@jarredsumner.com> * Add remix guide * Fix title * add util.formatWithOptions (#4090) * Add formatWithOptions * tests and tweaks * adjust --------- Co-authored-by: Jarred Sumner <jarred@jarredsumner.com> * bun test: format description of test.each (#4092) * bun test: format description * add tests for tests * only --------- Co-authored-by: Jarred Sumner <jarred@jarredsumner.com> * Fixes #4062 (#4106) * Fixes #4062 * Update encoding.zig * Use faster C++ impl * Update wtf-bindings.cpp * undo * Fixup --------- Co-authored-by: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com> * zig fmt * Update remix guide * fs.zig: create temp files with 0o700, not 0o007 (#4107) * Handle thundering herd of setInterval (#4109) Co-authored-by: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com> * Fix memory leak in base64url (#4111) Co-authored-by: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com> * run files without extensions (#4113) * run script without extension * process stdio write fix * don't check for trailing slash, var stream * More lazily initialize these static strings * Remove assertion * wip * Add --inspect flag * Deprecate loading `node_modules.bun` * realpath * regenerate schema * More * more * Update cli.zig * Debugger JS loads * have fun * Most of the code * Its starting to work. * more progress * win some, lose some * Update dap.ts * Async Context Tracking * untested websocket * Emit Console messages * Error handling * errors * Make profiling work better * [clap] CLI arguments with optional values ignore positional params In `bun --inspect foo.js`, `foo.js` should not be the value of `--inspect`. It is a positional parameter. `--inspect=foo` * Support multiple simultaneous clients, automatically unpause on disconnect * regenerate * Update Makefile * Update WebKit * Update cli.zig * Update InternalModuleRegistry+createInternalModuleById.h * BaseURL * Update WebKit * Add web-inspector-bun * Update build.ts * formatting, mostly * Update debugger.ts * Update InternalModuleRegistryConstants.h * wrap * Update test * Update test --------- Co-authored-by: Ashcon Partovi <ashcon@partovi.net> Co-authored-by: Dylan Conway <dylan.conway567@gmail.com> Co-authored-by: dave caruso <me@paperdave.net> Co-authored-by: Tiramify (A.K. Daniel) <94789999+TiranexDev@users.noreply.github.com> Co-authored-by: Jacques <25390037+jecquas@users.noreply.github.com> Co-authored-by: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com> Co-authored-by: Ai Hoshino <ambiguous404@gmail.com> Co-authored-by: Yash Sharma <yashosharma@gmail.com> Co-authored-by: Yash Sharma <yashsharma@Yashs-MacBook-Air.local> Co-authored-by: Colin McDonnell <colinmcd94@gmail.com> Co-authored-by: jhmaster <32803471+jhmaster2000@users.noreply.github.com> Co-authored-by: Adhityaa Chandrasekar <github@adtac.in> Co-authored-by: Dylan Conway <35280289+dylan-conway@users.noreply.github.com>

jecquas marked this pull request as draft August 7, 2023 21:00

Jarred-Sumner reviewed Aug 8, 2023

View reviewed changes

src/bun.js/test/jest.zig Show resolved Hide resolved

ghiscoding mentioned this pull request Aug 8, 2023

bun test #1825

Open

jecquas added 11 commits August 8, 2023 20:26

rename callback to func

9f8c64e

update testscope to handle function arguments

bbbc214

works

fcbb59a

big cleanup

8a962cb

works in debug, not release

3b51891

fix memory issue & update tests

d8e786f

catch & str test

ff6f532

write types for each() & switch tests to ts

2124fad

rm & typo

b7dc359

move some code around & support describe

6fb022a

review changes

a394aa0

jecquas force-pushed the jest-each branch from 5df318d to a394aa0 Compare August 8, 2023 18:37

jecquas marked this pull request as ready for review August 8, 2023 18:37

jecquas requested a review from Jarred-Sumner August 8, 2023 18:38

Jarred-Sumner reviewed Aug 9, 2023

View reviewed changes

Jarred-Sumner merged commit 63f58f4 into oven-sh:main Aug 9, 2023

jecquas deleted the jest-each branch August 9, 2023 06:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(bun:test) add support for test.each() and describe.each() #4047

feat(bun:test) add support for test.each() and describe.each() #4047

jecquas commented Aug 7, 2023

Jarred-Sumner Aug 8, 2023

Jarred-Sumner Aug 8, 2023

Jarred-Sumner Aug 8, 2023

Jarred-Sumner Aug 8, 2023

Jarred-Sumner Aug 8, 2023

Jarred-Sumner commented Aug 8, 2023

jecquas commented Aug 8, 2023

Jarred-Sumner Aug 9, 2023

Jarred-Sumner Aug 9, 2023

Jarred-Sumner commented Aug 9, 2023

-            while (idx < this.func_arg_size) {
+            for (this.func_args) |arg| {
+               arg.unprotect();
+             }


		pub const EachData = struct { strong: JSC.Strong, is_test: bool };

		fn eachBind(

feat(bun:test) add support for test.each() and describe.each() #4047

feat(bun:test) add support for test.each() and describe.each() #4047

Conversation

jecquas commented Aug 7, 2023

What does this PR do?

How did you verify your code works?

Jarred-Sumner Aug 8, 2023

Choose a reason for hiding this comment

Jarred-Sumner Aug 8, 2023

Choose a reason for hiding this comment

Jarred-Sumner Aug 8, 2023

Choose a reason for hiding this comment

Jarred-Sumner Aug 8, 2023

Choose a reason for hiding this comment

Jarred-Sumner Aug 8, 2023

Choose a reason for hiding this comment

Jarred-Sumner commented Aug 8, 2023

jecquas commented Aug 8, 2023

Jarred-Sumner Aug 9, 2023

Choose a reason for hiding this comment

Jarred-Sumner Aug 9, 2023

Choose a reason for hiding this comment

Jarred-Sumner commented Aug 9, 2023