-
-
Notifications
You must be signed in to change notification settings - Fork 724
chore: move get_test_services
function to biome_test_utils
crate
#7438
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
chore: move get_test_services
function to biome_test_utils
crate
#7438
Conversation
|
WalkthroughThe patch adds Possibly related PRs
Suggested labels
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Flag potential breaking changes that are not documented:
1. Identify changes to public APIs/exports, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints (including removed/renamed items and changes to types, required params, return values, defaults, or behavior).
2. Ignore purely internal/private changes (e.g., code not exported from package entry points or marked internal).
3. Verify documentation exists: a "Breaking Change" section in the PR description and updates to CHANGELOG.md. Please share your feedback with us on this Discord post. ✨ Finishing Touches
🧪 Generate unit tests
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 |
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.
Actionable comments posted: 1
🧹 Nitpick comments (4)
crates/biome_test_utils/src/lib.rs (4)
625-627
: Be explicit with defaults to avoid inference surprises.Make the empty-case construction of services explicit; it’s a touch clearer for readers and future refactors.
Apply:
- return JsAnalyzerServices::from((Default::default(), Default::default(), file_source)); + return JsAnalyzerServices::from(( + Arc::<ModuleGraph>::default(), + Arc::<ProjectLayout>::default(), + file_source, + ));
635-637
: Optionally normalise relative paths to an absolute root.Normalising to
/{path}
matches howxtask
builds test paths and makes module resolution predictable across callers.Apply:
- let path_buf = Utf8PathBuf::from(path); + let path_buf = if Utf8Path::new(path).is_absolute() { + Utf8PathBuf::from(path) + } else { + Utf8PathBuf::from("/").join(path) + };
658-660
: Don’tunimplemented!
on unknown manifests.Failing hard here will abort docs checks for benign files (e.g.
biome.json
,jsconfig.json
). Prefer ignoring unknown manifests.Apply:
- _ => unimplemented!("Unhandled manifest: {biome_path}"), + _ => { + // Unknown manifest type for the module graph: ignore gracefully. + }
614-672
: Nice extraction; consider tiny robustness tweaks.Overall move is tidy and de-duplicates logic. After the small safety fixes above, this helper should be reusable by the website without surprises. If you want, I can add a couple of focused tests (empty map, relative manifest path, multi-file graph).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lock
is excluded by!**/*.lock
and included by**
📒 Files selected for processing (3)
crates/biome_test_utils/Cargo.toml
(1 hunks)crates/biome_test_utils/src/lib.rs
(3 hunks)xtask/rules_check/src/lib.rs
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_test_utils/Cargo.toml
crates/biome_test_utils/src/lib.rs
**/*.toml
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format all TOML files before committing (just f, via taplo-cli)
Files:
crates/biome_test_utils/Cargo.toml
crates/**/Cargo.toml
📄 CodeRabbit inference engine (CONTRIBUTING.md)
crates/**/Cargo.toml
: In internal crates, reference internal dependencies with workspace = true
Use path dependencies for dev-dependencies in internal crates to avoid requiring published versions
Files:
crates/biome_test_utils/Cargo.toml
crates/*/Cargo.toml
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Place new crates under the crates/ directory
Files:
crates/biome_test_utils/Cargo.toml
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format all Rust source files before committing (just f)
Files:
crates/biome_test_utils/src/lib.rs
xtask/rules_check/src/lib.rs
🧠 Learnings (16)
📓 Common learnings
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:52.001Z
Learning: Applies to crates/biome_js_formatter/**/Cargo.toml : Add biome_js_formatter as a path dependency in Cargo.toml: biome_js_formatter = { version = "0.0.1", path = "../biome_js_formatter" }
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:53:15.299Z
Learning: Applies to crates/biome_service/../biome_lsp/src/server.tests.rs : Keep end-to-end LSP tests in biome_lsp’s server.tests.rs
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-05T09:13:58.901Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/**/src/**/*.rs : Set the language field in declare_lint_rule! to the most specific applicable language (js/jsx/ts/tsx)
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-05T09:13:58.901Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/lint/nursery/**/*.rs : Place all new rules in the nursery group under biome_js_analyze/lib/src/lint/nursery
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-05T09:13:58.901Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/**/src/**/*.rs : In declare_lint_rule! metadata, set version: "next"
📚 Learning: 2025-08-11T11:48:52.001Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:52.001Z
Learning: Applies to crates/biome_js_formatter/**/Cargo.toml : Add biome_js_formatter as a path dependency in Cargo.toml: biome_js_formatter = { version = "0.0.1", path = "../biome_js_formatter" }
Applied to files:
crates/biome_test_utils/Cargo.toml
📚 Learning: 2025-08-11T11:48:27.774Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:27.774Z
Learning: Applies to crates/biome_formatter/biome_html_formatter/Cargo.toml : Add the specified dev-dependencies (biome_formatter_test, biome_html_factory, biome_html_parser, biome_parser, biome_service, countme with feature enable, iai 0.1.1, quickcheck, quickcheck_macros, tests_macros) to Cargo.toml under [dev-dependencies]
Applied to files:
crates/biome_test_utils/Cargo.toml
📚 Learning: 2025-09-07T17:35:00.490Z
Learnt from: CR
PR: biomejs/biome#0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-09-07T17:35:00.490Z
Learning: Applies to Cargo.toml : Define shared workspace dependencies in the root Cargo.toml using the [workspace.dependencies] table
Applied to files:
crates/biome_test_utils/Cargo.toml
📚 Learning: 2025-08-11T11:53:15.299Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:53:15.299Z
Learning: Applies to crates/biome_service/src/workspace.rs : Implement the Workspace trait in src/workspace.rs
Applied to files:
crates/biome_test_utils/Cargo.toml
xtask/rules_check/src/lib.rs
📚 Learning: 2025-09-07T17:35:00.490Z
Learnt from: CR
PR: biomejs/biome#0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-09-07T17:35:00.490Z
Learning: Applies to crates/**/Cargo.toml : In internal crates, reference internal dependencies with workspace = true
Applied to files:
crates/biome_test_utils/Cargo.toml
📚 Learning: 2025-08-11T11:53:15.299Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:53:15.299Z
Learning: Applies to crates/biome_service/src/workspace/watcher.tests.rs : Place watcher tests for workspace methods in src/workspace/watcher.tests.rs
Applied to files:
crates/biome_test_utils/Cargo.toml
xtask/rules_check/src/lib.rs
📚 Learning: 2025-08-17T08:55:30.118Z
Learnt from: CR
PR: biomejs/biome#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-17T08:55:30.118Z
Learning: Applies to crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/** : Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Applied to files:
crates/biome_test_utils/Cargo.toml
📚 Learning: 2025-09-05T09:13:58.901Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-05T09:13:58.901Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/**/src/**/*.rs : In declare_lint_rule! metadata, set version: "next"
Applied to files:
crates/biome_test_utils/Cargo.toml
📚 Learning: 2025-08-11T11:53:15.299Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:53:15.299Z
Learning: Applies to crates/biome_service/../biome_lsp/src/server.tests.rs : Keep end-to-end LSP tests in biome_lsp’s server.tests.rs
Applied to files:
crates/biome_test_utils/Cargo.toml
xtask/rules_check/src/lib.rs
📚 Learning: 2025-08-11T11:48:52.001Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:52.001Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Use dbg_write! to debug and inspect the written IR elements during formatter development
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-08-11T11:46:05.836Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_diagnostics/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:46:05.836Z
Learning: Applies to crates/biome_diagnostics/**/*.rs : Use helper advice types from biome_diagnostics::v2 (CodeFrameAdvice, CommandAdvice, DiffAdvice, LogAdvice) when suitable
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-09-05T09:13:58.901Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-05T09:13:58.901Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/**/src/**/*.rs : Prefer Box<[_]> over Vec<_> for Signals when emitting multiple diagnostics
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-08-11T11:46:05.836Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_diagnostics/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:46:05.836Z
Learning: Applies to crates/biome_diagnostics/**/*.rs : Types implementing Diagnostic must also implement Debug (e.g., use #[derive(Debug, Diagnostic)])
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-08-11T11:48:52.001Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:52.001Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Build an intermediate IR via the provided helper APIs when formatting
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-08-11T11:48:27.774Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:27.774Z
Learning: Applies to crates/biome_formatter/biome_html_formatter/tests/spec_test.rs : Create tests/spec_test.rs implementing the run(spec_input_file, _expected_file, test_directory, _file_type) function as shown and include!("language.rs")
Applied to files:
xtask/rules_check/src/lib.rs
🧬 Code graph analysis (2)
crates/biome_test_utils/src/lib.rs (4)
packages/@biomejs/js-api/src/wasm.ts (1)
BiomePath
(1-1)crates/biome_json_parser/src/lib.rs (1)
parse_json
(23-26)crates/biome_lsp/src/server.rs (1)
default
(540-542)crates/biome_module_graph/tests/snap/mod.rs (1)
new
(21-27)
xtask/rules_check/src/lib.rs (1)
crates/biome_test_utils/src/lib.rs (2)
get_added_paths
(199-220)get_test_services
(621-672)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (24)
- GitHub Check: Parser conformance
- GitHub Check: Test Node.js API
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Check Dependencies
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Documentation
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Bench (biome_graphql_formatter)
- GitHub Check: Bench (biome_module_graph)
- GitHub Check: Bench (biome_json_parser)
- GitHub Check: Bench (biome_graphql_parser)
- GitHub Check: Bench (biome_configuration)
- GitHub Check: Bench (biome_package)
- GitHub Check: Bench (biome_html_formatter)
- GitHub Check: Bench (biome_html_parser)
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Bench (biome_css_parser)
- GitHub Check: Bench (biome_json_formatter)
- GitHub Check: Bench (biome_json_analyze)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: Bench (biome_css_formatter)
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: autofix
🔇 Additional comments (7)
crates/biome_test_utils/Cargo.toml (1)
25-25
: Dependency addition looks right.Adding
biome_js_analyze = { workspace = true }
aligns with our workspace policy and the new usage inget_test_services
.crates/biome_test_utils/src/lib.rs (4)
10-11
: New imports are appropriate.
MemoryFileSystem
andJsAnalyzerServices
are required for the new helper. All good.
14-14
: Importingparse_json
is correct.Needed for manifest parsing in the new helper.
27-27
: HashMap import: OK.Used by the public helper signature.
30-30
: BuildHasher import: OK.Keeps the helper generic over the hasher.
xtask/rules_check/src/lib.rs (2)
29-29
: Importing the shared helper is the right call.Good consolidation; keeps
xtask
lean.
29-29
: Resolved: onlybiome_test_utils::get_test_services
is in use No other local definitions or re-exports ofget_test_services
were found across the Rust sources.
path_buf.parent().unwrap().into(), | ||
&parsed.syntax().as_send().unwrap(), | ||
); | ||
} |
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.
🛠️ Refactor suggestion
Avoid unwrap on parent() to prevent panics with relative paths.
path_buf.parent().unwrap()
will panic for relative paths like "package.json"
. We already use a safer pattern elsewhere in this file; mirror it here.
Apply:
- layout.insert_serialized_node_manifest(
- path_buf.parent().unwrap().into(),
- &parsed.syntax().as_send().unwrap(),
- );
+ let dir = path_buf.parent().map(|p| p.to_path_buf()).unwrap_or_default();
+ layout.insert_serialized_node_manifest(
+ dir,
+ &parsed.syntax().as_send().unwrap(),
+ );
- layout.insert_serialized_tsconfig(
- path_buf.parent().unwrap().into(),
- &parsed.syntax().as_send().unwrap(),
- );
+ let dir = path_buf.parent().map(|p| p.to_path_buf()).unwrap_or_default();
+ layout.insert_serialized_tsconfig(
+ dir,
+ &parsed.syntax().as_send().unwrap(),
+ );
Also applies to: 654-656
🤖 Prompt for AI Agents
In crates/biome_test_utils/src/lib.rs around lines 642-645 (and also apply the
same fix at 654-656), replace the unconditional path_buf.parent().unwrap() with
a safe check: obtain the parent via if let Some(parent) = path_buf.parent() {
use parent.into() } else { return/continue/handle the no-parent case the same
way used elsewhere in this file (e.g., skip this insertion or use
Path::new(".").into() as appropriate) } so the code no longer panics for
relative paths like "package.json".
CodSpeed Performance ReportMerging #7438 will not alter performanceComparing Summary
|
@ryan-m-walker Can you address clippy errors? |
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.
Just a few comments on the functions docs.
Co-authored-by: Arend van Beelen jr. <arend@arendjr.nl>
Co-authored-by: Arend van Beelen jr. <arend@arendjr.nl>
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.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
xtask/rules_check/src/lib.rs (1)
15-15
: Cull unused imports to silence ClippyThese are no longer referenced here.
-use biome_fs::{BiomePath, MemoryFileSystem}; +use biome_fs::BiomePath; - use biome_js_analyze::JsAnalyzerServices; - use biome_module_graph::ModuleGraph; - use biome_project_layout::ProjectLayout; - use std::sync::Arc;Also applies to: 17-17, 23-24, 36-36
♻️ Duplicate comments (1)
crates/biome_test_utils/src/lib.rs (1)
642-646
: Don’t unwrap() parent dirs or syntax handles; handle relative paths and parse failures gracefully
parent().unwrap()
will panic whenpath
has no parent (e.g."package.json"
).as_send().unwrap()
can also panic. Use safe fallbacks.- layout.insert_serialized_node_manifest( - path_buf.parent().unwrap().into(), - &parsed.syntax().as_send().unwrap(), - ); + let dir = path_buf.parent().map(|p| p.to_path_buf()).unwrap_or_default(); + if let Some(syntax) = parsed.syntax().as_send() { + layout.insert_serialized_node_manifest(dir, &syntax); + }- layout.insert_serialized_tsconfig( - path_buf.parent().unwrap().into(), - &parsed.syntax().as_send().unwrap(), - ); + let dir = path_buf.parent().map(|p| p.to_path_buf()).unwrap_or_default(); + if let Some(syntax) = parsed.syntax().as_send() { + layout.insert_serialized_tsconfig(dir, &syntax); + }Also applies to: 654-656
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lock
is excluded by!**/*.lock
and included by**
📒 Files selected for processing (3)
crates/biome_test_utils/Cargo.toml
(1 hunks)crates/biome_test_utils/src/lib.rs
(3 hunks)xtask/rules_check/src/lib.rs
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_test_utils/src/lib.rs
crates/biome_test_utils/Cargo.toml
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format all Rust source files before committing (just f)
Files:
crates/biome_test_utils/src/lib.rs
xtask/rules_check/src/lib.rs
**/*.toml
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format all TOML files before committing (just f, via taplo-cli)
Files:
crates/biome_test_utils/Cargo.toml
crates/**/Cargo.toml
📄 CodeRabbit inference engine (CONTRIBUTING.md)
crates/**/Cargo.toml
: In internal crates, reference internal dependencies with workspace = true
Use path dependencies for dev-dependencies in internal crates to avoid requiring published versions
Files:
crates/biome_test_utils/Cargo.toml
crates/*/Cargo.toml
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Place new crates under the crates/ directory
Files:
crates/biome_test_utils/Cargo.toml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: Test Node.js API
- GitHub Check: Bench (biome_graphql_formatter)
- GitHub Check: Bench (biome_package)
- GitHub Check: Bench (biome_module_graph)
- GitHub Check: Bench (biome_json_formatter)
- GitHub Check: Bench (biome_json_analyze)
- GitHub Check: Bench (biome_html_formatter)
- GitHub Check: Bench (biome_configuration)
- GitHub Check: Bench (biome_css_parser)
- GitHub Check: Bench (biome_html_parser)
- GitHub Check: Bench (biome_graphql_parser)
- GitHub Check: Bench (biome_css_formatter)
- GitHub Check: Bench (biome_json_parser)
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Parser conformance
- GitHub Check: autofix
🔇 Additional comments (1)
crates/biome_test_utils/Cargo.toml (1)
25-25
: LGTM: workspace dep added correctly
biome_js_analyze = { workspace = true }
matches the workspace dependency policy.
return JsAnalyzerServices::from((Default::default(), Default::default(), file_source)); | ||
} |
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.
🛠️ Refactor suggestion
Unify return types: wrap defaults in Arc for consistency
Avoid mixing tuple types for From
. Use Arc
in the empty-path fast path too.
- if files.is_empty() {
- return JsAnalyzerServices::from((Default::default(), Default::default(), file_source));
- }
+ if files.is_empty() {
+ let module_graph = Arc::new(ModuleGraph::default());
+ let layout = Arc::new(ProjectLayout::default());
+ return JsAnalyzerServices::from((module_graph, layout, file_source));
+ }
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
return JsAnalyzerServices::from((Default::default(), Default::default(), file_source)); | |
} | |
if files.is_empty() { | |
let module_graph = Arc::new(ModuleGraph::default()); | |
let layout = Arc::new(ProjectLayout::default()); | |
return JsAnalyzerServices::from((module_graph, layout, file_source)); | |
} |
🤖 Prompt for AI Agents
In crates/biome_test_utils/src/lib.rs around lines 627-628, the fast-path return
uses raw Default tuple instead of wrapping defaults in Arc like other branches;
change the returned tuple to wrap the default values in Arc (e.g.,
Arc::new(Default::default()) for the shared components) so the From conversion
sees the same Arc-wrapped types as the non-empty-path path, ensuring consistent
return types.
_ => unimplemented!("Unhandled manifest: {biome_path}"), | ||
} | ||
} else { |
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.
🛠️ Refactor suggestion
Replace unimplemented! with a no-op to avoid panics in test utilities
Unknown manifests shouldn’t blow up the runner; just ignore them (the file still lands in the in-memory FS below).
- _ => unimplemented!("Unhandled manifest: {biome_path}"),
+ _ => {
+ // Ignore unknown manifests in tests; keep them in the in-memory FS.
+ }
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
_ => unimplemented!("Unhandled manifest: {biome_path}"), | |
} | |
} else { | |
_ => { | |
// Ignore unknown manifests in tests; keep them in the in-memory FS. | |
} |
🤖 Prompt for AI Agents
In crates/biome_test_utils/src/lib.rs around lines 659-661, the match arm
currently calls unimplemented!("Unhandled manifest: {biome_path}") which causes
a panic for unknown manifests; replace that with a no-op so the runner ignores
unknown manifests (e.g., remove the unimplemented! call and leave the arm empty
or add a simple comment/log without panicking) so the file still lands in the
in-memory FS and tests do not abort.
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
xtask/rules_check/src/lib.rs (2)
533-539
: Sanity-check: wrong language passed to create_analyzer_options for CSS/GraphQLThese branches use JsonLanguage; they likely should use CssLanguage and GraphqlLanguage respectively.
- let options = create_analyzer_options::<JsonLanguage>( + let options = create_analyzer_options::<CssLanguage>( &workspace_settings, project_key, &file_path, test, );- let options = create_analyzer_options::<JsonLanguage>( + let options = create_analyzer_options::<GraphqlLanguage>( &workspace_settings, project_key, &file_path, test, );Also applies to: 579-585
341-343
: Minor: prefer borrowing when building BiomePathAvoids constructing by value; matches typical usage elsewhere.
- let path = BiomePath::new(Utf8PathBuf::from(&file_path)); + let path_buf = Utf8PathBuf::from(&file_path); + let path = BiomePath::new(&path_buf);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
xtask/rules_check/src/lib.rs
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format all Rust source files before committing (just f)
Files:
xtask/rules_check/src/lib.rs
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:53:15.299Z
Learning: Applies to crates/biome_service/../biome_lsp/src/server.tests.rs : Keep end-to-end LSP tests in biome_lsp’s server.tests.rs
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:53:15.299Z
Learning: Applies to crates/biome_service/src/workspace/watcher.tests.rs : Place watcher tests for workspace methods in src/workspace/watcher.tests.rs
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:52.001Z
Learning: Applies to crates/biome_js_formatter/**/Cargo.toml : Add biome_js_formatter as a path dependency in Cargo.toml: biome_js_formatter = { version = "0.0.1", path = "../biome_js_formatter" }
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-05T09:13:58.901Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/tests/quick_test.rs : Use quick_test.rs for ad-hoc testing; set RuleFilter to the rule (e.g., RuleFilter::Rule("nursery", "useAwesomeTrick")) and adjust SOURCE
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-05T09:13:58.901Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/lint/nursery/**/*.rs : Place all new rules in the nursery group under biome_js_analyze/lib/src/lint/nursery
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-05T09:13:58.901Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/**/src/**/*.rs : Set the language field in declare_lint_rule! to the most specific applicable language (js/jsx/ts/tsx)
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-05T09:13:58.901Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/**/src/**/*.rs : When porting a lint rule, set sources in declare_lint_rule! (RuleSource::Eslint(...).same() or .inspired())
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:50:12.090Z
Learning: Applies to crates/biome_js_type_info/{src,resolver,biome_module_graph/src}/**/*.rs : Implement and use type resolution via the TypeResolver trait; resolvers own TypeStore vectors and provide fast by-id and hashed lookups
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:53:15.299Z
Learning: Applies to crates/biome_service/src/workspace.rs : Implement the Workspace trait in src/workspace.rs
📚 Learning: 2025-08-11T11:53:15.299Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:53:15.299Z
Learning: Applies to crates/biome_service/../biome_lsp/src/server.tests.rs : Keep end-to-end LSP tests in biome_lsp’s server.tests.rs
Applied to files:
xtask/rules_check/src/lib.rs
📚 Learning: 2025-08-11T11:53:15.299Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:53:15.299Z
Learning: Applies to crates/biome_service/src/workspace/watcher.tests.rs : Place watcher tests for workspace methods in src/workspace/watcher.tests.rs
Applied to files:
xtask/rules_check/src/lib.rs
📚 Learning: 2025-08-17T08:55:30.118Z
Learnt from: CR
PR: biomejs/biome#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-17T08:55:30.118Z
Learning: Applies to crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/** : Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Applied to files:
xtask/rules_check/src/lib.rs
🧬 Code graph analysis (1)
xtask/rules_check/src/lib.rs (1)
crates/biome_test_utils/src/lib.rs (1)
get_test_services
(622-673)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (25)
- GitHub Check: Documentation
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Check Dependencies
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Bench (biome_html_parser)
- GitHub Check: Bench (biome_configuration)
- GitHub Check: Bench (biome_package)
- GitHub Check: Bench (biome_json_parser)
- GitHub Check: Test Node.js API
- GitHub Check: Bench (biome_html_formatter)
- GitHub Check: Bench (biome_module_graph)
- GitHub Check: Bench (biome_json_formatter)
- GitHub Check: Bench (biome_json_analyze)
- GitHub Check: Bench (biome_graphql_formatter)
- GitHub Check: Bench (biome_graphql_parser)
- GitHub Check: Bench (biome_css_parser)
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: Bench (biome_css_formatter)
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: autofix
- GitHub Check: Parser conformance
🔇 Additional comments (4)
xtask/rules_check/src/lib.rs (4)
26-26
: Switching to biome_test_utils::get_test_services is spot onGood consolidation; removes duplication and makes reuse easy. Nothing else to do here.
15-15
: BiomePath import is correctUsed in create_analyzer_options; all good.
20-20
: Narrowing to JsonParserOptions onlyClean import; avoids pulling parse_json unnecessarily.
10-10
: Don’t remove theConsole
import
TheConsole
trait is needed for theconsole.println(...)
calls onEnvConsole
, so the originaluse biome_console::{Console, markup};
must remain.Likely an incorrect or invalid review comment.
@siketyan done! |
…iomejs#7438) Co-authored-by: Arend van Beelen jr. <arend@arendjr.nl>
Summary
This PR moves the
get_test_services
function from the xtask to the `biome_test_utils crate so that it can be shared more easily with the Biomejs website repo in the codegen process.See this PR for reference
Test Plan
n/a
Docs
n/a