Skip to content

[PROVES EFT-3295 #1226] Abandon tests fail on main as it stands — do not merge#1241

Open
jimmyp wants to merge 1 commit into
mainfrom
jimpelletier/eft-3295-prove-abandon-missing-on-main
Open

[PROVES EFT-3295 #1226] Abandon tests fail on main as it stands — do not merge#1241
jimmyp wants to merge 1 commit into
mainfrom
jimpelletier/eft-3295-prove-abandon-missing-on-main

Conversation

@jimmyp
Copy link
Copy Markdown
Contributor

@jimmyp jimmyp commented May 28, 2026

Purpose

The two abandon integration tests from #1226, plus only the empty stubs needed to make them compile and run. Behaviour-bearing code is left as no-ops:

  • ScriptServiceV2.AbandonScriptAsync returns a status snapshot but never fires the abandon token.
  • RunningScript has no abandon-token catch block; the script process keeps running until it exits naturally or the cancel kills it.
  • The script-isolation mutex is NOT released on abandon.

The single real (non-stub) production change is the Hitman env-var check that makes the scenario reproducible: with TentacleDebugDisableProcessKill_UNSAFE_FOR_PRODUCTION set, cancel does not kill the script process, so the test reaches the abandon path instead of the cancel-just-killed-it path. Without that, the tests would not exercise the abandon contract at all.

Running the tests against this branch produces meaningful assertion failures that document the behaviour gap #1226 closes.

Test failures

Failed AbandonScript_WhenCancelFailsToKillProcess_ReturnsAbandonedExitCode(Listening,Latest,ScriptServiceV2) [33 s]
Error Message:
 System.Exception : Abandoned script did not reach Complete state within 30s

Failed AbandonScript_WhenCancelFailsToKillProcess_ReturnsAbandonedExitCode(Polling,Latest,ScriptServiceV2) [33 s]
Error Message:
 System.Exception : Abandoned script did not reach Complete state within 30s

Failed AbandonScript_ReleasesIsolationMutexEvenWhileProcessIsStillRunning(Listening,Latest,ScriptServiceV2) [2 m]
  (test hits IntegrationTestTimeout: the second FullIsolation script
   blocks waiting on a mutex the first script never released)

Failed AbandonScript_ReleasesIsolationMutexEvenWhileProcessIsStillRunning(Polling,Latest,ScriptServiceV2) [2 m]
  (test hits IntegrationTestTimeout: the second FullIsolation script
   blocks waiting on a mutex the first script never released)

Failed\!  - Failed: 4, Passed: 0, Skipped: 0, Total: 4

Files changed

File What's in it
ClientScriptExecutionAbandon.cs The two abandon integration tests (copied verbatim from #1226)
EnvironmentVariables.cs Adds the TentacleDebugDisableProcessKill_UNSAFE_FOR_PRODUCTION constant
SilentProcessRunner.cs Hitman.TryKillProcessAndChildrenRecursively early-returns when the env var is set, so the test can reproduce a "cancel can't kill" state
ScriptExitCodes.cs Adds AbandonedExitCode = -48 (the value the test asserts on)
AbandonScriptCommandV2.cs New contract DTO, no logic
IScriptServiceV2.cs / IAsyncClientScriptServiceV2.cs / IAsyncScriptServiceV2.cs Adds the AbandonScript method signature to the three V2 interfaces so the contract compiles
ScriptServiceV2.cs Server-side AbandonScriptAsync STUB: returns the current status snapshot WITHOUT firing any abandon token. This is the behaviour gap.
ITentacleClient.cs / TentacleClient.cs Client-side AbandonScript method wired through the RPC executor; downstream is the no-op above
TentacleClientExtensionMethods.cs Test-only helpers that let the test call tentacleClient.AbandonScript(scriptTicket, CT) etc. with a ScriptTicket instead of a CommandContext
ScriptServiceV2DecoratorBuilder.cs / TestDecoratorsAreCalledInTheCorrectOrder.cs Test scaffolding extended to implement the new interface method (pass-through / NotImplementedException)

What's deliberately NOT here (and is on #1226):

  • The abandon-token catch block in RunningScript that translates the token into AbandonedExitCode
  • The workspace.Delete tolerance in ScriptServiceV2.CompleteScriptAsync
  • The CapabilitiesServiceV2 advertisement of AbandonScriptV2
  • The RunningScript.CreateAbandonable factories
  • Test refactors and the SilentProcessRunner async migration

Not for merge

#1226 is the real PR with the real implementation. Close this one when #1226 lands.

@jimmyp jimmyp marked this pull request as ready for review May 28, 2026 04:13
@jimmyp jimmyp requested review from a team as code owners May 28, 2026 04:13
@jimmyp jimmyp mentioned this pull request May 28, 2026
@jimmyp jimmyp changed the title [PROVES EFT-3295 #1226] Abandon tests don't compile on main — do not merge [PROVES EFT-3295 #1226] Minimum abandon implementation: tests pass on this branch — do not merge May 28, 2026
This branch carries the two abandon integration tests plus only the
stubs needed to make them compile and run. Behaviour-bearing types are
left as no-ops:
- AbandonScript RPC declarations exist but the server impl returns a
  status snapshot without firing any abandon token.
- RunningScript has no abandon-token handling; the script process keeps
  running until it exits naturally or the cancel kills it.
- The isolation mutex is NOT released when the script abandons.

The one real (non-stub) production change is the Hitman env-var check
that makes the test scenario reproducible: with
TentacleDebugDisableProcessKill_UNSAFE_FOR_PRODUCTION set, cancel does
not kill the script process, so the test reaches the abandon path
instead of the cancel-just-killed-it path.

Tests run and fail with assertion errors that document the gap:
- AbandonScript_WhenCancelFailsToKillProcess_ReturnsAbandonedExitCode
  fails with "Abandoned script did not reach Complete state within 30s"
- AbandonScript_ReleasesIsolationMutexEvenWhileProcessIsStillRunning
  times out at IntegrationTestTimeout (the second script blocks
  forever waiting on the mutex the first script never released).

This PR is not for merge.
@jimmyp jimmyp force-pushed the jimpelletier/eft-3295-prove-abandon-missing-on-main branch from 34dc3c0 to 812e53a Compare May 28, 2026 04:52
@jimmyp jimmyp changed the title [PROVES EFT-3295 #1226] Minimum abandon implementation: tests pass on this branch — do not merge [PROVES EFT-3295 #1226] Abandon tests fail on main as it stands — do not merge May 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant