Nest test results under describe scopes #12189

jakeboone02 · 2024-06-27T01:59:01Z

What does this PR do?

Updates the output of bun test so that describe scope names are only printed once and test/it results are nested and indented beneath their respective scopes.

Fixes Nest test/it calls under a single describe label instead of repeating the describe label for each test #3342
Documentation or TypeScript types
Code changes

How did you verify your code works?

There were no existing automated tests for the output format, so manual inspection has been the only test so far. I can write some tests if necessary.

I included a test for the new code, or existing tests cover it
I ran my tests locally and they pass (bun-debug test test-file-name.test)

Comparison of results

import { describe, expect, it } from "bun:test";

describe("our first test group", () => {
  describe("sub group 1", () => {
    it("knows zero is zero", () => { expect(0).toBe(0); });
    it("1 is the loneliest number", () => { expect(1).toBe(1); });
  });
  describe("nested group 2", () => {
    it("likes the number 2", () => { expect(2).toBe(2); });
    it.todo("thing 3 is still to do", () => { expect(3).toBe(0); });
  });
  it("here's thing 4", () => { expect(4).toBe(4); });
});

describe("some more tests", () => {
  it("does thing cinco", () => { expect(5).toBe(5); });
  it.skip("skips thing 6", () => { expect(6).toBe(6); });
  it("does thing {# days of the week}", () => { expect(7).toBe(7); });
});

mangs · 2024-06-27T02:36:02Z

@jakeboone02 This is so nice! Great job! One thing that I noticed while looking this over was the "new" example shows as being over 6x slower than before. Is that repeatable/consistent?

jakeboone02 · 2024-06-27T02:39:25Z

@jakeboone02 This is so nice! Great job! One thing that I noticed while looking this over was the "new" example shows as being over 6x slower than before. Is that repeatable/consistent?

Thanks! I think the speed difference is just because the new one is a debug build. I'll try a release build later tonight.

github-actions · 2024-06-27T02:54:37Z

❌ @jakeboone02, your commit has failing tests :(

💪 1 failing tests Darwin AARCH64

test/js/web/streams/streams.test.js 1 failing

🪟💻 1 failing tests Windows x64 baseline

test/integration/next-pages/test/dev-server.test.ts 1 failing

View logs

jakeboone02 · 2024-06-27T06:40:40Z

@mangs I tried the release build from this PR with bunx bun-pr 12189 and the performance was basically equivalent (a tiny bit faster for some reason??):

$ hyperfine --warmup 50 'bun test ./bun-debug-test.test.ts' 'bun-12189 test ./bun-debug-test.test.ts'
Benchmark 1: bun test ./bun-debug-test.test.ts
  Time (mean ± σ):      12.5 ms ±   2.2 ms    [User: 8.3 ms, System: 8.3 ms]
  Range (min … max):     9.9 ms …  36.4 ms    234 runs

  Warning: Statistical outliers were detected. Consider re-running this benchmark on a quiet PC without any interferences from other programs. It might help to use the '--warmup' or '--prepare' options.

Benchmark 2: bun-12189 test ./bun-debug-test.test.ts
  Time (mean ± σ):      11.8 ms ±   1.6 ms    [User: 7.5 ms, System: 8.5 ms]
  Range (min … max):     9.5 ms …  23.4 ms    209 runs

  Warning: Statistical outliers were detected. Consider re-running this benchmark on a quiet PC without any interferences from other programs. It might help to use the '--warmup' or '--prepare' options.

Summary
  'bun-12189 test ./bun-debug-test.test.ts' ran
    1.06 ± 0.23 times faster than 'bun test ./bun-debug-test.test.ts'

@Jarred-Sumner I have a feeling this speed difference is insignificant and/or inconsistent, but I'd love to know if I accidentally made it faster somehow.

EDIT: after some more benchmarking, it does appear to have made very little perf impact, if any.

jakeboone02

I added commit c2c667a that better aligns the message for when a .todo test passes and the --todo flag has been set. I didn't really know what I was doing so I copied similar code from here:

bun/src/cli/test_command.zig

Lines 113 to 122 in d356e27

    
           fn printTestLine(label: string, elapsed_ns: u64, parent: ?*jest.DescribeScope, comptime skip: bool, writer: anytype) void { 
        
               var scopes_stack = std.BoundedArray(*jest.DescribeScope, 64).init(0) catch unreachable; 
        
               var parent_ = parent; 
        
               while (parent_) |scope| { 
        
                   scopes_stack.append(scope) catch break; 
        
                   parent_ = scope.parent; 
        
               } 
        
               const scopes: []*jest.DescribeScope = scopes_stack.slice();

That seems like a lot of code just to get the number of items in a linked list, so please let me know if there's a more efficient way.

src/bun.js/test/jest.zig

dylan-conway

Nice, I like this.

Doesn't need to happen in this pr, but I think printing a status indicator for describe scopes would also be an improvement. Vitest does this, it makes the scopes look a little less out of place

jest for comparison:

jakeboone02 · 2024-06-28T21:57:35Z

Yeah, I had the same thought. Needs statusing at the file level, too. I think bun test kind of prints the test results as it goes through them, so it might require some refactoring to get the overall status of each file/describe and then print everything.

Other than that, is this ready after I commit that suggestion about the BoundedArray?

dylan-conway · 2024-06-28T22:13:13Z

Yeah, I had the same thought. Needs statusing at the file level, too. I think bun test kind of prints the test results as it goes through them, so it might require some refactoring to get the overall status of each file/describe and then print everything.

Yeah, we will probably need to store the results and print either after a describe scope finishes or maybe after the file finishes.

Other than that, is this ready after I commit that suggestion about the BoundedArray?

Yep I'd say it's good to go after that

dylan-conway

Thanks!

This reverts commit 6a43f7f.

dylan-conway · 2024-07-11T04:07:55Z

We will be reverting this pr before releasing v1.1.19. It's a step in the right direction, but we want to spend more time refining the output. Currently there is too little distinction between file name and describe scope name

jakeboone02 · 2024-07-11T06:11:05Z

@dylan-conway I noticed that too, which is one reason I wanted to work on #12378. I'd still like to work on that and get the aesthetics right.

Is that something you guys want to do in-house, or could I maybe propose a design? Probably a mixture of the Vitest and Jest output styles.

This reverts commit 6a43f7f.

Nest test results under describe scopes

04517a0

Better alignment for "todo test passed" message

c2c667a

jakeboone02 commented Jun 27, 2024

View reviewed changes

Jarred-Sumner assigned dylan-conway Jun 27, 2024

Jarred-Sumner requested a review from dylan-conway June 27, 2024 23:25

dylan-conway reviewed Jun 28, 2024

View reviewed changes

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

dylan-conway reviewed Jun 28, 2024

View reviewed changes

jakeboone02 and others added 2 commits June 28, 2024 15:46

Remove unnecessary BoundedArray in jest.zig

5b76717

Merge branch 'main' into nested-describe-3342

5c9a897

dylan-conway approved these changes Jul 4, 2024

View reviewed changes

Merge branch 'main' into nested-describe-3342

dbef693

dylan-conway merged commit 6a43f7f into oven-sh:main Jul 4, 2024

jakeboone02 deleted the nested-describe-3342 branch July 5, 2024 07:22

This was referenced Jul 5, 2024

Report aggregate, worst-case test results at file and describe-scope levels #12378

Open

Failed test error messages should be printed at the end of the output #2109

Open

dylan-conway added a commit that referenced this pull request Jul 11, 2024

Revert "Nest test results under describe scopes (#12189)"

25f7ef7

This reverts commit 6a43f7f.

dylan-conway mentioned this pull request Jul 11, 2024

Nest test/it calls under a single describe label instead of repeating the describe label for each test #3342

Open

paperclover pushed a commit that referenced this pull request Jul 16, 2024

Revert "Nest test results under describe scopes (#12189)"

53a22da

This reverts commit 6a43f7f.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Nest test results under describe scopes #12189

Nest test results under describe scopes #12189

jakeboone02 commented Jun 27, 2024

mangs commented Jun 27, 2024 •

edited

Loading

jakeboone02 commented Jun 27, 2024

github-actions bot commented Jun 27, 2024

jakeboone02 commented Jun 27, 2024 •

edited

Loading

jakeboone02 left a comment •

edited

Loading

dylan-conway left a comment

jakeboone02 commented Jun 28, 2024

dylan-conway commented Jun 28, 2024

dylan-conway left a comment

dylan-conway commented Jul 11, 2024

jakeboone02 commented Jul 11, 2024

	fn printTestLine(label: string, elapsed_ns: u64, parent: ?*jest.DescribeScope, comptime skip: bool, writer: anytype) void {
	var scopes_stack = std.BoundedArray(*jest.DescribeScope, 64).init(0) catch unreachable;
	var parent_ = parent;

	while (parent_) \|scope\| {
	scopes_stack.append(scope) catch break;
	parent_ = scope.parent;
	}

	const scopes: []*jest.DescribeScope = scopes_stack.slice();

Nest test results under describe scopes #12189

Nest test results under describe scopes #12189

Conversation

jakeboone02 commented Jun 27, 2024

What does this PR do?

How did you verify your code works?

Comparison of results

mangs commented Jun 27, 2024 • edited Loading

jakeboone02 commented Jun 27, 2024

github-actions bot commented Jun 27, 2024

💪 1 failing tests Darwin AARCH64

🪟💻 1 failing tests Windows x64 baseline

jakeboone02 commented Jun 27, 2024 • edited Loading

jakeboone02 left a comment • edited Loading

Choose a reason for hiding this comment

dylan-conway left a comment

Choose a reason for hiding this comment

jakeboone02 commented Jun 28, 2024

dylan-conway commented Jun 28, 2024

dylan-conway left a comment

Choose a reason for hiding this comment

dylan-conway commented Jul 11, 2024

jakeboone02 commented Jul 11, 2024

mangs commented Jun 27, 2024 •

edited

Loading

jakeboone02 commented Jun 27, 2024 •

edited

Loading

jakeboone02 left a comment •

edited

Loading