Skip to content

(#1031) Tests: wait for native Windows process termination - #1034

Open
pkazakov-dev wants to merge 3 commits into
ForNeVeR:mainfrom
pkazakov-dev:feature/test-fix-on-tests
Open

pkazakov-dev wants to merge 3 commits into
ForNeVeR:mainfrom
pkazakov-dev:feature/test-fix-on-tests

Conversation

@pkazakov-dev

@pkazakov-dev pkazakov-dev commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

WaitForExitAsync can observe an exit code through HasExited before the native Windows process handle is signaled. Wait for native termination before disposing the job object and allowing callers to delete test files, while retaining explicit command disposal.

Aims to close #1031

WaitForExitAsync can observe an exit code through HasExited before the native Windows process handle is signaled. Wait for native termination before disposing the job object and allowing callers to delete test files, while retaining explicit command disposal.

Extract the shared exit wait and add a Windows regression test using modern .NET child processes with many open native handles to expose the exit-code/termination window.

Validation: all three Cesium.TestFramework.Tests tests pass on .NET 10.0.14 with SDK 10.0.403. The regression assertion failed before adding the native wait.
public class ExecUtilTests(ITestOutputHelper output)
{
[WindowsFact]
public async Task ProcessHasTerminatedWhenExitWaitCompletes()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need tests for tests? Sounds fragile

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally, I am ok with having tests for test framework, and we actually write them.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, the question is then:
is this test needed in our suite?

I understand this test target specific regression as of now, but do we want to test this behavior?

@pkazakov-dev
pkazakov-dev marked this pull request as ready for review November 6, 2026 06:44
@ForNeVeR
ForNeVeR self-requested a review November 6, 2026 21:42
@ForNeVeR ForNeVeR self-assigned this Nov 6, 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.

Tests are unstable again

3 participants