Skip to content

Change xUnit RunspaceTests to run sequentially #8796

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jan 30, 2019
Merged

Change xUnit RunspaceTests to run sequentially #8796

merged 3 commits into from
Jan 30, 2019

Conversation

PaulHigin
Copy link
Contributor

@PaulHigin PaulHigin commented Jan 30, 2019

PR Summary

XUnit parallel runspace tests are failing with obscure errors. This appears to be due to parallelism and the tests appear to pass when run serially. This change is to convert the RunspaceTests to Sequential operation

PR Context

These tests have been failing fairly consistently if randomly in our daily builds, and appear to be due to running them in parallel.

PR Checklist

@iSazonov
Copy link
Collaborator

If we turn off this test we will never fix the race condition which is very bad for real world scenarios. I suggest to review and merge #8779 that will eliminate inter-thread race conditions. Then we could think more about inter-process race conditions. I already look this and couldl push a PR.

@adityapatwardhan
Copy link
Member

@PaulHigin Could you look at the xUnit failure?
Also put a description and context in the PR description

@PaulHigin
Copy link
Contributor Author

@adityapatwardhan @iSazonov The tests are still failing (but in a different test), and in the same way (malformed file path). So I will re-enable the test and try running as sequential instead of parallel, to verify this is indeed a race condition.

@PaulHigin PaulHigin changed the title Temporarily skipping randomly failing test WIP Temporarily skipping randomly failing test Jan 30, 2019
@PaulHigin PaulHigin changed the title WIP Temporarily skipping randomly failing test Change xUnit RunspaceTests to run sequentially Jan 30, 2019
@adityapatwardhan
Copy link
Member

Not waiting the recommended 24 hours since this is blocking PRs.

@adityapatwardhan adityapatwardhan merged commit f3efcab into PowerShell:master Jan 30, 2019
@adityapatwardhan adityapatwardhan added the CL-Test Indicates that a PR should be marked as a test change in the Change Log label Jan 30, 2019
@PaulHigin PaulHigin deleted the skip-parallel-runspacetest branch January 30, 2019 23:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CL-Test Indicates that a PR should be marked as a test change in the Change Log
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants