-
Notifications
You must be signed in to change notification settings - Fork 68
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
Fix flakey XCTest parallel test failure reporting #1343
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
d3e750a
to
be22745
Compare
f9499d8
to
83901a4
Compare
award999
approved these changes
Feb 7, 2025
Converting this to draft so it doesn’t get merged yet. I’m still using it to track down the source of a test failure |
775944b
to
737bb7c
Compare
8a63d76
to
e719452
Compare
The `ParallelXCTestRunStateProxy` in `XCTestOutputParser.ts` wraps the test run state and only allows issues to be recorded using the terminal output of the test run. The remaining test run state is filled in with the xml xUnit output. This is because SwiftPM only dumps test output to the terminal if there is a failure. Using these two sources we can (almost) fully reconstruct a regular test run. When parsing XCTest output from the terminal failure messages might be broken up over multiple lines. In order to capture all these lines and group them in to a single failure message even if the message is broken up across terminal buffer chunks we maintain an `excess` variable on the TestRunState, which we check at the beginning of chunk parsing to know if we need to continue an error message. However, the `ParallelXCTestRunStateProxy` that wraps the test run state was not forwarding along the setting of `excess`. Instead `excess` was set on the proxy, which is recreated for every chunk of output parsed. This manifested as XCTests _sometimes_ not correctly reporting their error message when run in the Parallel test profile, instead reporting the message "Failed". Issue: swiftlang#1334
e719452
to
9bab194
Compare
matthewbastien
approved these changes
Feb 13, 2025
plemarquand
added a commit
to plemarquand/vscode-swift
that referenced
this pull request
Mar 3, 2025
When performing a parallel XCTest run we first parse the terminal output, then parse the xUnit XML output and merge the two data sets together to create the run results. This merging was not being tested at the unit test level, only at the integration test level, which lead to a subtle and occasional bug, swiftlang#1334 (fixed in swiftlang#1343). As a followup, update the existing XCTestOutputParser tests to test both regular and parallel test run output. xUnit XML is synthesized from the expected results and fed in to the `TestXUnitParser` to modify the test's `TestRunState` just as it is in the extension.
plemarquand
added a commit
that referenced
this pull request
Mar 4, 2025
When performing a parallel XCTest run we first parse the terminal output, then parse the xUnit XML output and merge the two data sets together to create the run results. This merging was not being tested at the unit test level, only at the integration test level, which lead to a subtle and occasional bug, #1334 (fixed in #1343). As a followup, update the existing XCTestOutputParser tests to test both regular and parallel test run output. xUnit XML is synthesized from the expected results and fed in to the `TestXUnitParser` to modify the test's `TestRunState` just as it is in the extension.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
ParallelXCTestRunStateProxy
inXCTestOutputParser.ts
wraps the test run state and only allows issues to be recorded using the terminal output of the test run. The remaining test run state is filled in with the xml xUnit output. This is because SwiftPM only dumps test output to the terminal if there is a failure. Using these two sources we can (almost) fully reconstruct a regular test run.When parsing XCTest output from the terminal failure messages might be broken up over multiple lines. In order to capture all these lines and group them in to a single failure message even if the message is broken up across terminal buffer chunks we maintain an
excess
variable on the TestRunState, which we check at the beginning of chunk parsing to know if we need to continue an error message.However, the
ParallelXCTestRunStateProxy
that wraps the test run state was not forwarding along the setting ofexcess
. Insteadexcess
was set on the proxy, which is recreated for every chunk of output parsed.This manifested as XCTests sometimes not correctly reporting their error message when run in the Parallel test profile, instead reporting the message "Failed".
Issue: #1334