Skip to content

Adopt ordered multi-caret markers in FCS tests (no behaviour change) - #20082

Open
T-Gro wants to merge 5 commits into
mainfrom
tests/adopt-ordered-carets
Open

Adopt ordered multi-caret markers in FCS tests (no behaviour change)#20082
T-Gro wants to merge 5 commits into
mainfrom
tests/adopt-ordered-carets

Conversation

@T-Gro

@T-Gro T-Gro commented Jul 24, 2026

Copy link
Copy Markdown
Member

Follow-up to #20033, building on @auduchinok's #20077. Adopts the ordered multi-caret / selection markers (SourceContext.extractOrderedMarkedSources + {selstart}/{selend}) to remove copy-pasted test sources and magic coordinates.

No behaviour change — every migrated test keeps identical assertions (each commit rubber-duck-verified byte-for-byte; tests green on net472 + net10.0).

Reductions:

  • Goto-def tests: drop unused open System + now-redundant caret-anchor (*loc-N*) comments
  • Completion ArraySliceNotation (3 source copies → 1), CurriedArguments (5 → 1), Bug312557_2 (4 → 1), Bug69673_1 (2 → 1)
  • Symbols Nested copy-and-update (8 Facts sharing a copied source → 1)
  • BreakpointLocation: magic ((line,col),(line,col)) tuples → {selstart}/{selend} markers in the source

Deliberately not migrated (it would make the code worse, not better):

  • Tooltip/Completion tests using markAtStartOfMarker/markAtEndOfMarker on a shared source — they already reuse the source, with heterogeneous asserts per position; {caretN} there is 0 reduction and messier.
  • $ / direct-column low-level marker tests (IdentifierIsland, QuickParse, …) — $ is more compact than {caret}; migrating adds indirection + a ±1 column risk for no reduction.

T-Gro and others added 2 commits July 24, 2026 09:30
…anchor comments

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 9d20cd22-45dc-411f-9163-185fd6dd54d9
Uses SourceContext.extractOrderedMarkedSources (multi-caret) to replace whole-source
[<Theory>] copies with one marked source in DotOff.ArraySliceNotation (3 copies) and
CurriedArguments.Regression (5 copies). No behaviour change.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 9d20cd22-45dc-411f-9163-185fd6dd54d9
@github-actions

github-actions Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

⚠️ Release notes required, but author opted out

Warning

Author opted out of release notes, check is disabled for this pull request.
cc @dotnet/fsharp-team-msft

T-Gro and others added 3 commits July 24, 2026 14:59
Bug312557_2 (4 source copies -> 1 {caretN}) and Bug69673_1.CtrlSpaceForThis
(2-row Theory -> 1). No behaviour change.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 9d20cd22-45dc-411f-9163-185fd6dd54d9
…caretN}

The 8 Facts shared an identical source copied per caret; now one {caret1..8}
source + a (field-name, range) cases list. No behaviour change.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 9d20cd22-45dc-411f-9163-185fd6dd54d9
…source

Replaces magic ((line,col),(line,col)) tuples with {selstart}/{selend} markers
around the breakpoint span; the validation caret is inferred from {selend}
(SourceContext), so no {caret} needed. Compares against context.SelectedRange.
No behaviour change (all 6 tests green).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 9d20cd22-45dc-411f-9163-185fd6dd54d9
@T-Gro
T-Gro force-pushed the tests/adopt-ordered-carets branch from 54ac48a to b8c97d2 Compare July 24, 2026 13:46
@T-Gro
T-Gro marked this pull request as ready for review July 24, 2026 13:49
@T-Gro
T-Gro requested a review from a team as a code owner July 24, 2026 13:49
@T-Gro T-Gro added the NO_RELEASE_NOTES Label for pull requests which signals, that user opted-out of providing release notes label Jul 24, 2026
@T-Gro
T-Gro requested a review from abonie July 24, 2026 13:52
@github-actions github-actions Bot added AI-Tooling-Check-Bypassed Tooling check: non-fork PR, not diff-analyzed AI-Tooling-Check-Scanned-Clean Tooling check: diff analyzed, no interesting infrastructure files labels Jul 24, 2026
@auduchinok

auduchinok commented Jul 28, 2026

Copy link
Copy Markdown
Member

Deliberately not migrated (it would make the code worse, not better):
Tooltip/Completion tests using markAtStartOfMarker/markAtEndOfMarker on a shared source — they already reuse the source, with heterogeneous asserts per position; {caretN} there is 0 reduction and messier.
$ / direct-column low-level marker tests (IdentifierIsland, QuickParse, …) — $ is more compact than {caret}; migrating adds indirection + a ±1 column risk for no reduction.

Should we convert them all, so more tests could use the same infrastructure and the extra helpers could be removed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI-Tooling-Check-Bypassed Tooling check: non-fork PR, not diff-analyzed AI-Tooling-Check-Scanned-Clean Tooling check: diff analyzed, no interesting infrastructure files NO_RELEASE_NOTES Label for pull requests which signals, that user opted-out of providing release notes

Projects

Status: New

Development

Successfully merging this pull request may close these issues.

2 participants