-
Notifications
You must be signed in to change notification settings - Fork 5.1k
console.assert prints Assertion failed prefix with messages 🤖🤖🤖 #42155
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| console.assert(false); | ||
| console.assert(false, "message"); | ||
| console.assert(false, "with args", 1, true); | ||
| console.assert(true, "should not print"); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| import { join } from "node:path"; | ||
| import { expect, it } from "bun:test"; | ||
| import { bunEnv, bunExe } from "harness"; | ||
|
|
||
| it("console.assert prints 'Assertion failed' and prefixes messages like Node.js", () => { | ||
| const filepath = join(import.meta.dir, "console-assert.fixture.js").replaceAll("\\", "/"); | ||
| const proc = Bun.spawnSync({ | ||
| cmd: [bunExe(), filepath], | ||
| stdin: "inherit", | ||
| stdout: "pipe", | ||
| stderr: "pipe", | ||
| env: bunEnv, | ||
| }); | ||
| expect(proc.exitCode).toBe(0); | ||
| const stdout = proc.stdout.toString("utf8").replaceAll("\r\n", "\n"); | ||
| const stderr = proc.stderr.toString("utf8").replaceAll("\r\n", "\n"); | ||
| expect(stdout).toBe(""); | ||
| expect(stderr).toContain("Assertion failed\n"); | ||
| expect(stderr).toContain("Assertion failed: message\n"); | ||
| expect(stderr).toContain("Assertion failed: with args 1 true\n"); | ||
|
Comment on lines
+18
to
+20
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win Assert the complete stderr contract.
Based on the stated Proposed fix- expect(stderr).toContain("Assertion failed\n");
- expect(stderr).toContain("Assertion failed: message\n");
- expect(stderr).toContain("Assertion failed: with args 1 true\n");
- expect(stderr).not.toContain("should not print");
+ expect(stderr).toBe(
+ "Assertion failed\n" +
+ "Assertion failed: message\n" +
+ "Assertion failed: with args 1 true\n",
+ );🤖 Prompt for AI Agents |
||
| expect(stderr).not.toContain("should not print"); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Assert output before
proc.exitCode.If the fixture exits with a nonzero code, this assertion stops the test before it checks
stdoutandstderr. Move the exit-code assertion after all output assertions.Based on learnings: Bun subprocess tests must assert stdout and stderr before checking the subprocess exit code, with the exit-code assertion last.
🤖 Prompt for AI Agents
Source: Learnings