Fix parallel BAML test flakiness and aggregate reporting - #3828
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
⏭️ Performance benchmarks were skippedPerf benchmarks (CodSpeed) are opt-in on pull requests — they no longer run on every push. They always run automatically after merge to To run them on this PR, do any of the following, then push a commit (or re-run CI):
|
|
No description provided. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
baml_language/crates/baml_cli/tests/exit_code_e2e.rs (1)
387-423: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer unit tests for helper-level parsing/summary logic, keep E2E as smoke.
This new E2E case is valuable, but the newly added pure logic in
test_command.rs(leaf summary / failed-name extraction) is better covered with unit tests to reduce brittleness and runtime cost.As per coding guidelines,
**/*.rs: Prefer writing Rust unit tests over integration tests where possible.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/crates/baml_cli/tests/exit_code_e2e.rs` around lines 387 - 423, The test_unfiltered_testset_run_reports_failed_child_name function in exit_code_e2e.rs is testing pure parsing and summary logic that should be covered by unit tests instead. Extract the pure logic for leaf summary calculation and failed child name extraction from test_command.rs into dedicated unit test functions, and refactor this E2E test to be a minimal smoke test that only verifies the command runs successfully and returns the expected exit code without asserting detailed output formatting. This reduces brittleness and test runtime while keeping E2E tests focused on end-to-end workflows.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@baml_language/crates/baml_cli/tests/exit_code_e2e.rs`:
- Around line 387-423: The test_unfiltered_testset_run_reports_failed_child_name
function in exit_code_e2e.rs is testing pure parsing and summary logic that
should be covered by unit tests instead. Extract the pure logic for leaf summary
calculation and failed child name extraction from test_command.rs into dedicated
unit test functions, and refactor this E2E test to be a minimal smoke test that
only verifies the command runs successfully and returns the expected exit code
without asserting detailed output formatting. This reduces brittleness and test
runtime while keeping E2E tests focused on end-to-end workflows.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b834a260-3d80-42b8-acff-af4738154cc9
⛔ Files ignored due to path filters (10)
baml_language/crates/baml_cli/src/snapshots/baml_cli__describe_command_tests__render_testing_package_listing.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/fs.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/glob.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__testing_std__/baml_tests__compiles____testing_std____03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__testing_std__/baml_tests__compiles____testing_std____04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__testing_std__/baml_tests__compiles____testing_std____04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__testing_std__/baml_tests__compiles____testing_std____06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/tests/bytecode_format/snapshots/bytecode_format__bytecode_display_expanded.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/tests/bytecode_format/snapshots/bytecode_format__bytecode_display_expanded_unoptimized.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/tests/bytecode_format/snapshots/bytecode_format__bytecode_display_textual.snapis excluded by!**/*.snap
📒 Files selected for processing (7)
baml_language/crates/baml_builtins2/baml_std/testing/registry.bamlbaml_language/crates/baml_builtins2/baml_std/testing/runners.bamlbaml_language/crates/baml_builtins2/baml_std/testing/types.bamlbaml_language/crates/baml_cli/src/test_command.rsbaml_language/crates/baml_cli/tests/exit_code_e2e.rsbaml_language/crates/baml_tests/baml_src/ns_fs/fs.bamlbaml_language/crates/baml_tests/baml_src/ns_glob/glob.baml
1 participant
Summary
Verification
Stacked on #3772.
Summary by CodeRabbit
New Features
Bug Fixes