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.
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 howxtaskbuilds 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.lockis excluded by!**/*.lockand 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.tomlcrates/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.rsxtask/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.tomlxtask/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.tomlxtask/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.tomlxtask/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.
MemoryFileSystemandJsAnalyzerServicesare required for the new helper. All good.
14-14: Importingparse_jsonis 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
xtasklean.
29-29: Resolved: onlybiome_test_utils::get_test_servicesis in use No other local definitions or re-exports ofget_test_serviceswere found across the Rust sources.
| path_buf.parent().unwrap().into(), | ||
| &parsed.syntax().as_send().unwrap(), | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ 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? |
arendjr
left a comment
There was a problem hiding this comment.
Just a few comments on the functions docs.
Co-authored-by: Arend van Beelen jr. <[email protected]>
Co-authored-by: Arend van Beelen jr. <[email protected]>
There was a problem hiding this comment.
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 whenpathhas 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.lockis excluded by!**/*.lockand 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.rscrates/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.rsxtask/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.
🛠️ 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.
🛠️ 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.
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 theConsoleimport
TheConsoletrait 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. <[email protected]>
Summary
This PR moves the
get_test_servicesfunction 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