Skip to content

[Repo Assist] test: re-enable Go-to-implementation-on-interface-definition test - #1506

Merged
Krzysztof-Cieslak merged 2 commits into
mainfrom
repo-assist/fix-issue-986-goto-implementation-604562b087f1624d
Mar 18, 2026
Merged

Krzysztof-Cieslak merged 2 commits into
mainfrom
repo-assist/fix-issue-986-goto-implementation-604562b087f1624d

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

🤖 This is an automated pull request from Repo Assist.

Closes #986.

Problem

The Go-to-implementation-on-interface-definition test was disabled with ptestCaseAsync (skipped) since PR #977, with TODO comments and commented-out range assertions. The root cause of the original disabling was a bug in the test's match pattern: the server returns U2.C1(U2.C2 locations) for multiple Location[] results, but the test matched against U2.C2 at the top level — which would only match LocationLink[] responses. This caused the test to always hit U2.C1 _res -> failtest "Should be multiple GotoResult" and fail.

Fix

  • ptestCaseAsync → testCaseAsync to re-enable the test
  • Fix the match arms to correctly unwrap U2.C1(U2.C2 res) for a multiple-location response
  • Add meaningful assertions: at least 2 results, all in Library.fs
  • Remove the stale TODO comments and incorrect hard-coded range assertions (which had wrong character offsets)

Trade-offs

The new assertions validate the essential behaviour (the feature works and returns implementations from the right file) without over-specifying exact byte offsets, which makes the test more resilient to minor FCS range changes.

Test Status

Build succeeded. GoTo test suite ran and the previously-skipped test now passes (38 passed, 2 unrelated infrastructure timeouts on network-dependent sourcelink tests, 4 C#-dependency skips).

Generated by Repo Assist · ◷

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@346204513ecfa08b81566450d7d599556807389f

…oses #986)

The test was disabled (ptestCaseAsync) with TODO comments. Root cause: the
match pattern was wrong — the server returns U2.C1(U2.C2 locations) for
multiple results (Location[]) but the test checked for U2.C2 at the top
level (which would be LocationLink[]).

Fix:
- Change ptestCaseAsync -> testCaseAsync to re-enable the test
- Fix match pattern to properly unwrap U2.C1(U2.C2 ...)
- Add assertions: >=2 results, all in Library.fs
- Remove stale TODO comments and incorrect range assertions

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Krzysztof-Cieslak
Krzysztof-Cieslak marked this pull request as ready for review March 18, 2026 12:01
@Krzysztof-Cieslak
Krzysztof-Cieslak merged commit b405c41 into main Mar 18, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Re-enable Go-to-implementation-on-interface-definition test

1 participant