Skip to content
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/parsers/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1849,7 +1849,12 @@ mod tests {
out.push('\n');
}

writeln!(out, "bool={:?}", Expr::get_boolean(&root, b"private")).unwrap();
writeln!(
out,
"bool={:?}",
root.get(b"private").and_then(|e| e.as_bool())
)
.unwrap();
writeln!(out, "num={:?}", root.get_number(b"count").map(|(n, _)| n)).unwrap();
writeln!(
out,
Expand Down
21 changes: 21 additions & 0 deletions src/parsers/native_test_shims.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,24 @@ unsafe extern "C" fn highway_index_of_any_char(
};
t.iter().position(|c| cs.contains(c)).unwrap_or(text_len)
}

#[unsafe(no_mangle)]
unsafe extern "C" fn highway_memmem(
haystack: *const u8,
haystack_len: usize,
needle: *const u8,
needle_len: usize,
) -> *const u8 {
// SAFETY: caller contract of `bun_highway::memmem` guarantees both
// (ptr, len) pairs describe valid readable ranges.
let (h, n) = unsafe {
(
core::slice::from_raw_parts(haystack, haystack_len),
core::slice::from_raw_parts(needle, needle_len),
)
};
h.windows(n.len())
.position(|w| w == n)
// SAFETY: `i < haystack_len`, so `haystack.add(i)` stays in bounds.
.map_or(core::ptr::null(), |i| unsafe { haystack.add(i) })
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
7 changes: 3 additions & 4 deletions src/sys/file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -486,13 +486,12 @@ pub(crate) mod tests {
#[test]
fn dropping_stdio_is_safe() {
let _g = FD_TEST_LOCK.lock();
// `File::stdin()` / `stdout()` / `stderr()` wrap process-shared
// descriptors that the caller does not own. Dropping the wrapper must
// not tear down the test harness's output.
// `File::stdin()` / `stdout()` wrap process-shared descriptors that the
// caller does not own. Dropping the wrapper must not tear down the test
// harness's output.
for _ in 0..16 {
let _ = File::stdin();
let _ = File::stdout();
Comment thread
robobun marked this conversation as resolved.
Outdated
let _ = File::stderr();
}
assert!(fstat(Fd::stdout()).is_ok());
}
Expand Down
46 changes: 46 additions & 0 deletions test/internal/rust-tests-compile.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// Verifies that the `#[cfg(test)]` code in a fixed set of Rust crates compiles.
//
// `bun bd`, `rust:check`, and `rust:clippy` all build with the default cfg only,
// so a workspace-wide refactor that removes a symbol whose sole caller sits
// behind `#[cfg(test)]` is invisible to every other CI lane. That is exactly how
// #35002 broke `cargo test -p bun_parsers` (removed `Expr::get_boolean`) and
// `cargo test -p bun_sys` (removed `File::stderr`).
//
// This does not try to link or run the unit tests (linking needs the C/C++ dep
// archive assembled by `scripts/bench-json-rust.sh`); `cargo check --tests` is
// enough to catch the class of breakage above and is fast on a warm build tree.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
import { expect, test } from "bun:test";
import { existsSync } from "node:fs";
import { join } from "node:path";

const repo = join(import.meta.dir, "..", "..");

// Crates whose `#[cfg(test)]` modules are worth guarding here. Keep this list
// small: each entry costs a `cargo check` of its dependency closure.
const CRATES = ["bun_parsers", "bun_sys"];

const cargo = Bun.which("cargo");
// `cargo check` resolves the workspace, which requires the codegen dir that
// `bun bd` / `bun run build --configure-only` populates.
const workspaceReady = existsSync(join(repo, "build", "debug", "codegen", "build_options.rs"));

Check warning on line 25 in test/internal/rust-tests-compile.test.ts

View check run for this annotation

Claude / Claude Code Review

workspaceReady skip check omits vendor/lolhtml

The `workspaceReady` skip check only guards on `build/debug/codegen/build_options.rs`, but cargo cannot resolve the workspace unless `vendor/lolhtml/Cargo.toml` also exists (root `Cargo.toml` has `lol_html = { path = "vendor/lolhtml" }`). The sibling `test/internal/linear-fifo.test.ts` already checks both for exactly this reason ("test-only lanes run a prebuilt binary and lack vendor/lolhtml"); add `&& existsSync(join(repo, "vendor", "lolhtml", "Cargo.toml"))` here to match.
Comment thread
robobun marked this conversation as resolved.
Outdated

test.skipIf(!cargo || !workspaceReady)(
`cargo check --tests compiles: ${CRATES.join(", ")}`,
async () => {
await using proc = Bun.spawn({
cmd: [cargo!, "check", ...CRATES.flatMap(c => ["-p", c]), "--tests", "--keep-going", "--message-format=short"],
cwd: repo,
env: { ...process.env, CARGO_TERM_COLOR: "never" },
stdout: "pipe",
stderr: "pipe",
});
const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]);

Check warning on line 37 in test/internal/rust-tests-compile.test.ts

View check run for this annotation

Claude / Claude Code Review

stdout piped but never drained

`stdout: "pipe"` is set but `proc.stdout` is never drained — only `proc.stderr.text()` and `proc.exited` are awaited. Per REVIEW.md's subprocess-test rule ("drain pipes concurrently … an unread pipe fills the ~64KB OS buffer and deadlocks the child"), either change to `stdout: "ignore"` since the test never inspects it, or add `proc.stdout.text()` to the `Promise.all`.
Comment thread
robobun marked this conversation as resolved.
Outdated
const errors = stderr
.split("\n")
.filter(l => l.includes(": error[") || l.includes(": error:"))
.join("\n");
expect(errors).toBe("");
expect(exitCode).toBe(0);
},
120_000,
);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Loading