Fix flakey WhenScriptServiceIsRunning_ThenWorkspaceIsNotDeleted test #745
Conversation
|
[sc-67411] |
|
|
||
| runningScriptResult.LogExecuteScriptOutput(Logger); | ||
|
|
||
| runningScriptResult.ScriptExecutionResult.ExitCode.Should().Be(0, "Script should have completed successfully"); |
There was a problem hiding this comment.
We could keep the tests to ensure that runningScriptTask succeeded (as it was the Directory.Exists(startScriptWorkspaceDirectory) check that failed).
I know it's much like Directory.Exists(startScriptWorkspaceDirectory), and technically not part of what's being tested. But still 😁
There was a problem hiding this comment.
I only recently added those asserts into the test to see if there was something else causing the test to fail!! Log / assert as much as possible to try and track down the issue (which is by design 😆 )
If the script fails or not should not really matter for the intent of the test. We await the execute script operation so the orchestration of the script should have been successful which is all we really care about
nathanwoctopusdeploy
deleted the
sast/fix-flakey-WhenScriptServiceIsRunning_ThenWorkspaceIsNotDeleted
branch
Background
The test
WhenScriptServiceIsRunning_ThenWorkspaceIsNotDeletedwas written to ensure that a running script did not have it's workspace cleaned up by the background cleanup job while it was still running / not completed.The test then went on to verify that the workspace was cleaned up when CompleteScript was called. This turns out to be somewhat flakey as the
CompleteScriptcall does a best effort attempt to delete the workspace before silently failing!!While it would be nice to have a reliable test that asserts that complete script always cleans up the workspace it seems the code to try and silently fail is by design and the intent of this test was to make sure the workspace isn't unexpectedly deleted by the background cleanup job.
Test has been modified to remove the assertion that CompleteScript cleans up the workspace.
How to review this PR
Quality ✔️
Pre-requisites