Skip to content

Launch the viewer without the test host's handles, and settle by value - #895

Closed
SimonCropp wants to merge 1 commit into
mainfrom
fix-library
Closed

SimonCropp wants to merge 1 commit into
mainfrom
fix-library

Conversation

@SimonCropp

Copy link
Copy Markdown
Member

This fixes the last two "Library and inline" items from todo.md, which #894 left open because each needed a design decision. That section of todo.md is now empty and removed.

dotnet test does not return until the launched viewer is closed

The viewer inherited the pipe dotnet test reads the test host's output from, and dotnet test reads it until every writer has closed it.

  • Windows: the launch uses ShellExecute, which inherits no handles. A process started without it inherits every inheritable handle, whatever is redirected, which is why redirecting all three streams still hung. Measured with a stand-in host and a 15 s child: 16.2 s to end of output before, 1.2 s after.
  • Linux and macOS: .NET opens every descriptor close-on-exec, so only the standard streams pass to a child. All three are redirected and this side's ends are closed.
  • The patch: a ShellExecute launch cannot redirect stdin, so the patch goes in a temp file named by --payload. The viewer reads the file and deletes it. With no --payload, it still reads stdin, so the documented < the.inlinepatch form and an older DiffEngine keep working.
  • Windows head: it is now a WinExe. The SDK only infers that for a -windows framework, so the apphost was a console executable, and ShellExecute would have given it a console window beside the viewer. It attaches to its parent's console only to print a usage error.

Tests: ViewerLauncherTests.OnWindowsTheViewerInheritsNothing, ElsewhereTheViewerHasStandardStreamsOfItsOwn, AnInlineLaunchNamesItsPayloadFile, and CommandLineTests.InlineWithPayload, APayloadFileIsReadAndDeleted, AMissingPayloadFileIsNoPayload. The stand-in host measurement is not a committed test: a real one would open a console window during the run.

Settle-by-member counts queue entries, not call sites

A settle can now carry the passing call's value, through a new optional value argument on DiffRunner.SettleInline and a value field on the wire. When the line names no entry, the member fallback then only takes an entry that value settles (InlinePatch.IsSettledBy): one anchored to it, or one waiting to become it. A value also picks one entry out of several in a member. InlineStaging.Clear takes the same value for the same fallback on disk.

Without a value, behaviour is unchanged, so nothing regresses until Verify passes one. An owner that predates the field skips it, as it skips any unknown field.

Tests: InlineQueueTests.SettleFromAPassingSiblingKeepsTheFailingSiblingsEntry, SettleByMemberTakesAnEntryAnchoredToTheValue, SettleByMemberTakesAnEntryWaitingForTheValue, SettleByMemberWithAValueChoosesAmongSeveral, ASettleValueRoundTrips, and InlineStagingTests.ClearFromAPassingSiblingKeepsTheFailingSiblingsStagedTrio, ClearByMemberTakesACallSiteTheValueSettles.

Verify needs a follow-up: pass the expected value to SettleInline (after SnapshotValue for F#) and to InlineStaging.Clear.

The full solution passes locally in Release: 2,154 passed, 22 skipped.

- MemberLine prefers a declaration whose span holds the hint, the outermost
  where several do, over one that is merely nearer. A same-named test in the
  next nested type, or an F# local named like the test, put the floor past the
  hint.
- NextMemberLine compares indentation as columns, with tab stops of four. A
  tab indented body under a space indented member ended the member at its
  first local.
- F# escapes that are malformed or not F#'s (\d, \0, \12, \e, \x4, \u12) keep
  the backslash as text, as fsi does, rather than making the literal
  unreadable. A trigraph past 255 wraps into the byte range, as fsi does.
- An F# regular literal is read as the snapshot it holds, with the layout
  convention applied as the test library applies it, and layout-shaped content
  that has to be written as a regular literal is wrapped in layout of its own.
  The patcher called such a literal already applied, so it was never updated.
- WildcardFileFinder finds nothing, rather than throwing, under a root that
  does not exist: an undefined Program Files variable threw out of DiffTools'
  static constructor.
- ViewerLaunchGate.LaunchAsync runs the launch on the pool and does not
  capture the caller's context while it holds the gate, so a sync Launch
  behind it on a single threaded context no longer deadlocks.
- InlineApplier answers Failed where opening the mutex or the patcher throws,
  rather than unwinding a viewer's loop.
- Staged file names cut the test name to fit 255 bytes, and on .NET Framework
  the whole path to MAX_PATH. Longer names were silently not staged.
- A reply on the viewer port that is not the protocol marks the port unowned
  and says once which variable moves DiffEngine off it, rather than being
  taken for an owner that silently took nothing.
@SimonCropp SimonCropp closed this Sep 23, 2026
@SimonCropp
SimonCropp deleted the fix-library branch September 23, 2026 07:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant