Finding
Two independent pieces of unused/duplicated test infrastructure in crates/mcpls-core/tests/common/:
1. mock_lsp.rs is entirely dead code. MockLspServer, MockBehaviorBuilder, MockLspChannel (116 lines) are declared pub and every item is wrapped in #[allow(dead_code)] — the annotation itself is evidence nothing constructs them. A repo-wide search confirms zero call sites for MockLspServer::new, MockBehaviorBuilder::new, or MockLspChannel::new anywhere under tests/. The project's actual LSP-facing tests (ra_e2e.rs, integration/rust_analyzer_tests.rs) exercise a real rust-analyzer process instead, so this channel-based mock harness was superseded but never removed.
2. skip_if_no_rust_analyzer! is defined but never invoked, while its exact body is hand-duplicated 19 times. tests/common/test_utils.rs:28 defines:
macro_rules! skip_if_no_rust_analyzer {
() => {
if !$crate::common::test_utils::rust_analyzer_available() {
eprintln!("Skipping test: rust-analyzer not available");
return;
}
};
}
A repo-wide search shows zero invocations of this macro. Meanwhile tests/integration/rust_analyzer_tests.rs repeats the equivalent 3-line guard by hand at 19 separate call sites (lines 149, 195, 236, 281, 322, 366, 407, 454, 500, 557, 603, 647, 690, 716, 737, 772, 818, 857, 891), e.g.:
if !rust_analyzer_available() {
eprintln!("Skipping: rust-analyzer not available");
return;
}
(message text drifted slightly from the macro's — "Skipping" vs "Skipping test" — which is itself a symptom of the copies no longer having one source of truth.)
Location
crates/mcpls-core/tests/common/mock_lsp.rs (whole file)
crates/mcpls-core/tests/common/test_utils.rs:27-35 (macro definition)
crates/mcpls-core/tests/integration/rust_analyzer_tests.rs (19 duplicated guard blocks)
Why
Dead test scaffolding and un-DRY'd boilerplate both cost real maintenance time without adding coverage: mock_lsp.rs is compiled into the integration_tests test binary for zero benefit, and any future tweak to the skip message/behavior (e.g. adding a MCPLS_SKIP_RA-style opt-out like ra_e2e.rs already has) requires touching 19 sites instead of one. Either delete mock_lsp.rs, or wire it into a test that needs channel-based LSP mocking; and replace the 19 hand-rolled guards with skip_if_no_rust_analyzer!().
Finding
Two independent pieces of unused/duplicated test infrastructure in
crates/mcpls-core/tests/common/:1.
mock_lsp.rsis entirely dead code.MockLspServer,MockBehaviorBuilder,MockLspChannel(116 lines) are declaredpuband every item is wrapped in#[allow(dead_code)]— the annotation itself is evidence nothing constructs them. A repo-wide search confirms zero call sites forMockLspServer::new,MockBehaviorBuilder::new, orMockLspChannel::newanywhere undertests/. The project's actual LSP-facing tests (ra_e2e.rs,integration/rust_analyzer_tests.rs) exercise a realrust-analyzerprocess instead, so this channel-based mock harness was superseded but never removed.2.
skip_if_no_rust_analyzer!is defined but never invoked, while its exact body is hand-duplicated 19 times.tests/common/test_utils.rs:28defines:A repo-wide search shows zero invocations of this macro. Meanwhile
tests/integration/rust_analyzer_tests.rsrepeats the equivalent 3-line guard by hand at 19 separate call sites (lines 149, 195, 236, 281, 322, 366, 407, 454, 500, 557, 603, 647, 690, 716, 737, 772, 818, 857, 891), e.g.:(message text drifted slightly from the macro's — "Skipping" vs "Skipping test" — which is itself a symptom of the copies no longer having one source of truth.)
Location
crates/mcpls-core/tests/common/mock_lsp.rs(whole file)crates/mcpls-core/tests/common/test_utils.rs:27-35(macro definition)crates/mcpls-core/tests/integration/rust_analyzer_tests.rs(19 duplicated guard blocks)Why
Dead test scaffolding and un-DRY'd boilerplate both cost real maintenance time without adding coverage:
mock_lsp.rsis compiled into theintegration_teststest binary for zero benefit, and any future tweak to the skip message/behavior (e.g. adding aMCPLS_SKIP_RA-style opt-out likera_e2e.rsalready has) requires touching 19 sites instead of one. Either deletemock_lsp.rs, or wire it into a test that needs channel-based LSP mocking; and replace the 19 hand-rolled guards withskip_if_no_rust_analyzer!().