Skip to content
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

Do not process code coverage when tests failed #6894

Merged
merged 1 commit into from Sep 14, 2023

Conversation

neonichu
Copy link
Member

This can currently lead to bogus output like

error: terminated(1): /Users/neonacho/Downloads/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/llvm-profdata merge -sparse -o /Users/neonacho/Desktop/lunch7/.build/arm64-apple-macosx/debug/codecov/default.profdata output:
    error: no input files specified. See llvm-profdata merge -help

I would also think even if it succeeded, it could result in incorrect data.

rdar://115436056

This can currently lead to bogus output like

```
error: terminated(1): /Users/neonacho/Downloads/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/llvm-profdata merge -sparse -o /Users/neonacho/Desktop/lunch7/.build/arm64-apple-macosx/debug/codecov/default.profdata output:
    error: no input files specified. See llvm-profdata merge -help
```

I would also think even if it succeeded, it could result in incorrect data.

rdar://115436056
@neonichu
Copy link
Member Author

@swift-ci please test

@@ -296,7 +296,7 @@ public struct SwiftTestTool: SwiftCommand {
swiftTool.executionStatus = .failure
}

if self.options.enableCodeCoverage {
if self.options.enableCodeCoverage, ranSuccessfully {
Copy link
Member

Choose a reason for hiding this comment

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

Can this behavior be tested to avoid regressions?

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably, but I don't see any existing testing of the code-coverage feature at all.

@neonichu
Copy link
Member Author

Going to merge this.

I think it would be nice to have test coverage of the code coverage feature, but this doesn't necessarily seem like the place to start adding that.

@neonichu neonichu merged commit f700420 into main Sep 14, 2023
5 checks passed
@neonichu neonichu deleted the do-not-process-code-coverage-if-tests-failed branch September 14, 2023 20:53
MaxDesiatov pushed a commit that referenced this pull request Sep 28, 2023
This can currently lead to bogus output like

```
error: terminated(1): /Users/neonacho/Downloads/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/llvm-profdata merge -sparse -o /Users/neonacho/Desktop/lunch7/.build/arm64-apple-macosx/debug/codecov/default.profdata output:
    error: no input files specified. See llvm-profdata merge -help
```

I would also think even if it succeeded, it could result in incorrect data.

rdar://115436056
MaxDesiatov pushed a commit that referenced this pull request Sep 28, 2023
This can currently lead to bogus output like

```
error: terminated(1): /Users/neonacho/Downloads/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/llvm-profdata merge -sparse -o /Users/neonacho/Desktop/lunch7/.build/arm64-apple-macosx/debug/codecov/default.profdata output:
    error: no input files specified. See llvm-profdata merge -help
```

I would also think even if it succeeded, it could result in incorrect data.

rdar://115436056
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants