fix(ci): add #[ignore] to ra_e2e_suite and install rust-analyzer in e2e job#126
Merged
Conversation
…2e CI job Without #[ignore], ra_e2e_suite was not picked up by the CI e2e filter (--run-ignored ignored-only). Also adds rustup component add rust-analyzer step on Linux/macOS; Windows skips the ra_e2e suite via MCPLS_SKIP_RA=1.
…d code_action timeout - ra_probe: fall back to `rustup which rust-analyzer` when the binary is not in PATH — fixes CI on Windows where rustup toolchain dirs are excluded from PATH - sc_get_code_actions: increase retry loop from 15s to 30s and add 5s initial warm-up sleep; type-checking must complete before code action fixes appear - compute ca_end_col dynamically from the actual line length instead of a hardcoded column
- path_to_uri: use Url::from_file_path instead of manual string formatting; on Windows, canonicalize() returns \\?\ prefix which manual replace produced invalid file:////?\... URIs — causing all LSP requests to fail with -32603 - sc_get_code_actions: extend range to ca_line+1 because rust-analyzer reports the missing-semicolon diagnostic at the closing brace (one line below), so the range must overlap that position to trigger the quickfix - wait_until_ready: default timeout 120s on Windows (vs 60s on Linux/macOS) - ci: bump test-e2e timeout-minutes from 15 to 20
…king tests Reverts to manual URI construction but strips the extended-path prefix (\\?\) that canonicalize() adds on Windows. Using Url::from_file_path panicked on unit tests that pass Unix-style paths (/test/file.rs) which are invalid Windows absolute paths.
Add codeActionLiteralSupport to LSP client capabilities so rust-analyzer returns CodeAction objects instead of falling back to legacy Command-only format. Without this capability declaration RA yields an empty response for structural refactoring assists such as "Implement missing members". Update sc_get_code_actions to use a point-cursor range on the impl line and target the empty `impl Greet for CodeActionTarget` block. Fix sc_get_cached_diagnostics to poll up to 15 s for the push-based publishDiagnostics cache, which may arrive asynchronously after pull-based diagnostics have already been satisfied by sc_get_diagnostics.
…hed-diag timeout Readiness gate now requires 3 consecutive hover successes to avoid false-positive from transient RA analysis during workspace indexing (observed on Windows CI). sc_get_cached_diagnostics timeout raised to 60 s on Windows where publishDiagnostics notifications arrive later due to slower CI runners.
Url::from_file_path on a canonicalized \?\-prefixed Windows path produces a URI that does not match what rust-analyzer stores in publishDiagnostics notifications (which omit the \?\ prefix). Replace the Url::from_file_path call in handle_cached_diagnostics with path_to_uri, which strips \?\ before building the URI, keeping the cache lookup key consistent with the stored key.
The URI mismatch fix (path_to_uri instead of Url::from_file_path) means the push cache lookup now works on Windows. The previous 60 s Windows timeout combined with the ~32 s readiness gate was hitting the 120 s nextest terminate-after limit. 20 s is sufficient: if the cache is populated (URI fix works) the data is found on the first poll; if not, we fail clearly within budget.
Newer rust-analyzer versions skip publishDiagnostics push for files that were already served via the pull-based textDocument/diagnostic API. sc_get_diagnostics calls pull on broken.rs, so RA no longer pushes for it. lib.rs is opened only via hover (wait_until_ready), never via pull diagnostics, so RA unconditionally pushes publishDiagnostics for it after initial workspace indexing. lib.rs also contains a guaranteed E0425 error (undefined_variable in has_error) that RA always reports.
…A's URI normalization rust-analyzer normalizes Windows drive letters to lowercase in file URIs (e.g. C: → c:). path_to_uri was preserving the uppercase letter from canonicalize(), causing push-diagnostic cache lookups to miss every time on Windows CI — the stored key (RA's URI) and the lookup key (our URI) differed only in drive-letter case.
Different tools (rust-analyzer, std::fs::canonicalize) produce Windows file URIs with inconsistent drive-letter casing (C: vs c:). Instead of assuming one canonical form, normalize both sides to lowercase at the cache boundary via uri_cache_key(). Applies to store, get, and clear operations in NotificationCache.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up fixes for #125 that were pushed after the squash merge:
ra_e2e_suitewas missing#[ignore]— without it the CI e2e filter (--run-ignored ignored-only -E 'kind(test) and test(e2e)') skipped the new suite entirely, leaving only the original 11 tests runningrustup component add rust-analyzerstep to thetest-e2eCI job on all platforms (including Windows)Test plan
test-e2ejob now shows 12 tests (11 existing +ra_e2e_suite)cargo nextest run --workspace --all-features --lib --bins— 380 tests pass