test(language_server): refactor tester to use WorkspaceWorker #10730
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
github-actions
bot
added
the
C-test
CodSpeed Instrumentation Performance ReportMerging #10730 will not alter performanceComparing Summary
|
Sysix
force-pushed
the
04-30-test_language_server_refactor_tester_to_use_workspaceworker
branch
3 times, most recently
from
8ded402 to
8e91c38
Compare
Sysix
force-pushed
the
04-30-test_language_server_refactor_tester_to_use_workspaceworker
branch
from
8e91c38 to
4cd629a
Compare
graphite-app
bot
changed the base branch from
04-30-test_language_server_add_test_for_init_nested_configs_
to
graphite-base/10730
|
Just aligned the filenames to the fixture's directory. |
Merge activity
|
Sysix
force-pushed
the
04-30-test_language_server_refactor_tester_to_use_workspaceworker
branch
from
4cd629a to
6ddfc78
Compare
WalkthroughThis set of changes refactors the testing infrastructure and configuration handling in the 📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (15)
📒 Files selected for processing (9)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (6)
🧰 Additional context used🧬 Code Graph Analysis (1)crates/oxc_language_server/src/tester.rs (1)
⏰ Context from checks skipped due to timeout of 90000ms (9)
🔇 Additional comments (18)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
crates/oxc_language_server/src/linter/server_linter.rs (1)
31-31: Matches visibility issue flagged inmain.rs
use crate::tester::Tester;relies on thetestermodule being public.
Ensure the fix proposed inmain.rsis applied; otherwise this import will not resolve.
🧹 Nitpick comments (2)
crates/oxc_language_server/src/main.rs (1)
19-19: Remove stray “// #” placeholder commentThe comment does not convey any useful context and clutters the import section.
Consider deleting it to keep the header clean.-// #crates/oxc_language_server/src/tester.rs (1)
96-103: PreferPathBuf::joinover manualformat!for cross-platform safetyUsing string concatenation risks introducing mixed separators on Windows vs Unix.
Since you already convert the root directory to aPathBuf, join the file component there.- let uri = get_file_uri(&format!("{}/{}", self.relative_root_dir, relative_file_path)); + let uri = get_file_uri( + &std::path::Path::new(self.relative_root_dir) + .join(relative_file_path) + .to_string_lossy(), + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (15)
crates/oxc_language_server/src/linter/snapshots/fixtures_linter_cross_module_nested_config_dep-a.ts.snapis excluded by!**/*.snapcrates/oxc_language_server/src/linter/snapshots/fixtures_linter_hello_world.js@no_errors.snapis excluded by!**/*.snapcrates/oxc_language_server/src/snapshots/[email protected]is excluded by!**/*.snapcrates/oxc_language_server/src/snapshots/[email protected]is excluded by!**/*.snapcrates/oxc_language_server/src/snapshots/[email protected]is excluded by!**/*.snapcrates/oxc_language_server/src/snapshots/[email protected]is excluded by!**/*.snapcrates/oxc_language_server/src/snapshots/[email protected]is excluded by!**/*.snapcrates/oxc_language_server/src/snapshots/fixtures_linter_cross_module_nested_config@folder_folder-dep-a.ts.snapis excluded by!**/*.snapcrates/oxc_language_server/src/snapshots/fixtures_linter_deny_no_console@hello_world.js.snapis excluded by!**/*.snapcrates/oxc_language_server/src/snapshots/[email protected]is excluded by!**/*.snapcrates/oxc_language_server/src/snapshots/[email protected]is excluded by!**/*.snapcrates/oxc_language_server/src/snapshots/fixtures_linter_no_errors@hello_world.js.snapis excluded by!**/*.snapcrates/oxc_language_server/src/snapshots/[email protected]is excluded by!**/*.snapcrates/oxc_language_server/src/snapshots/[email protected]is excluded by!**/*.snapcrates/oxc_language_server/src/snapshots/[email protected]is excluded by!**/*.snap
📒 Files selected for processing (9)
crates/oxc_language_server/fixtures/linter/deny_no_console/.oxlintrc.json(1 hunks)crates/oxc_language_server/fixtures/linter/no_errors/.oxlintrc.json(1 hunks)crates/oxc_language_server/fixtures/linter/no_errors/hello_world.js(1 hunks)crates/oxc_language_server/fixtures/linter/regexp_feature/.oxlintrc.json(1 hunks)crates/oxc_language_server/src/linter/mod.rs(0 hunks)crates/oxc_language_server/src/linter/server_linter.rs(1 hunks)crates/oxc_language_server/src/main.rs(1 hunks)crates/oxc_language_server/src/tester.rs(2 hunks)crates/oxc_language_server/src/worker.rs(1 hunks)
💤 Files with no reviewable changes (1)
- crates/oxc_language_server/src/linter/mod.rs
🧰 Additional context used
🧬 Code Graph Analysis (1)
crates/oxc_language_server/src/worker.rs (1)
crates/oxc_language_server/src/tester.rs (1)
get_file_uri(13-17)
⏰ Context from checks skipped due to timeout of 90000ms (14)
- GitHub Check: Test (ubuntu-latest)
- GitHub Check: Test wasm32-wasip1-threads
- GitHub Check: Test (macos-latest)
- GitHub Check: Clippy
- GitHub Check: Benchmark (semantic)
- GitHub Check: Benchmark (formatter)
- GitHub Check: Test
- GitHub Check: Benchmark (minifier)
- GitHub Check: Benchmark (transformer)
- GitHub Check: Benchmark (isolated_declarations)
- GitHub Check: Benchmark (codegen)
- GitHub Check: Benchmark (parser)
- GitHub Check: Benchmark (lexer)
- GitHub Check: Build Linter Benchmark
🔇 Additional comments (5)
crates/oxc_language_server/src/worker.rs (1)
462-462: Update import path forget_file_uri
The test module now correctly importsget_file_urifrom the relocatedcrate::testermodule, reflecting the move of the testing utilities out oflinter. Make sure thetestermodule is exposed (e.g., viamod tester;under#[cfg(test)]) so this path resolves.crates/oxc_language_server/fixtures/linter/no_errors/.oxlintrc.json (1)
1-5: Add.oxlintrc.jsondisabling correctness rules
This fixture JSON correctly disables the "correctness" category for theno_errorsscenario. Structure and syntax are valid and align with the intended test configuration.crates/oxc_language_server/fixtures/linter/deny_no_console/.oxlintrc.json (1)
1-5: Add.oxlintrc.jsonenforcingno-consolerule
The configuration enables theno-consolerule with severity "error", which will properly catch console usage in tests. The JSON is well-formed.crates/oxc_language_server/fixtures/linter/no_errors/hello_world.js (1)
1-1: Introduce simple JavaScript fixture
A minimalconsole.log("Hello, world!");file is appropriate for theno_errorsfixture. It fulfills the test requirement under the disabled correctness rules. Optionally, ensure a trailing newline if your linter or formatter enforces it.crates/oxc_language_server/fixtures/linter/regexp_feature/.oxlintrc.json (1)
3-5: Enableno-unused-varsrule override
The update adds"no-unused-vars": "off"alongside existing regex lint rules. This ensures unused-variable warnings are suppressed in the regex feature tests. JSON formatting is correct.
The respect now each workspace-folder as an own workspace and lint like the user will expect it.
graphite-app
bot
force-pushed
the
04-30-test_language_server_refactor_tester_to_use_workspaceworker
branch
from
6ddfc78 to
9ebf3d4
Compare
graphite-app
bot
deleted the
04-30-test_language_server_refactor_tester_to_use_workspaceworker
branch
The respect now each workspace-folder as an own workspace and lint like the user will expect it.