Skip to content

Do not process code coverage when tests failed #6894

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 1 commit into from
Sep 14, 2023

Conversation

neonichu
Copy link
Contributor

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 neonichu self-assigned this Sep 13, 2023
@neonichu
Copy link
Contributor 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
Contributor

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
Contributor 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
Contributor 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
@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
grynspan added a commit that referenced this pull request Aug 1, 2024
…ail.

In #6894, code coverage processing was disabled because it produces spurious
output. This PR just adds a comment to the current source that links back to
that one for future reference.
grynspan added a commit that referenced this pull request Aug 1, 2024
…ail.

In #6894, code coverage processing was disabled because it produces spurious
output. This PR just adds a comment to the current source that links back to
that one for future reference.
grynspan added a commit that referenced this pull request Aug 1, 2024
…ail (#7844)

In #6894, code coverage processing was disabled because it produces
spurious output. This PR just adds a comment to the current source that
links back to that one for future reference.
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.

3 participants