[ECO-5252] Generate a code coverage report in CI#238
[ECO-5252] Generate a code coverage report in CI#238lawrence-forooghian merged 1 commit intomainfrom
Conversation
WalkthroughThis pull request adds code coverage generation capabilities to the CI pipeline. A new job called Changes
Sequence Diagram(s)sequenceDiagram
participant GitHubActions as GitHub Actions
participant BuildTool as BuildTool CLI
participant XcodeRunner as Xcode Runner
participant ReportGen as xcresulttool Action
GitHubActions->>GitHubActions: Trigger code-coverage job (after generate-matrices)
GitHubActions->>BuildTool: Execute "swift run BuildTool generate-code-coverage --result-bundle-path CodeCoverage.xcresult"
BuildTool->>XcodeRunner: Call runXcodebuild(action: "test", scheme: "AblyChat", destination: macOS, testPlan: <value>, resultBundlePath: CodeCoverage.xcresult)
XcodeRunner-->>BuildTool: Return test results and coverage data
BuildTool->>GitHubActions: Pass coverage data to workflow
GitHubActions->>ReportGen: Generate Markdown coverage report (runs on both success and failure)
Assessment against linked issues
Possibly related PRs
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
lawrence-forooghian
force-pushed
the
code-coverage
branch
from
b7d6331 to
ab63d8f
Compare
lawrence-forooghian
force-pushed
the
code-coverage
branch
from
ab63d8f to
e09cf27
Compare
Resolves #240.
lawrence-forooghian
force-pushed
the
code-coverage
branch
from
e09cf27 to
04d8565
Compare
lawrence-forooghian
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.github/workflows/check.yaml (1)
173-174:Details
❓ Verification inconclusive
Consider setting additional configuration options
The xcresulttool action appears to support additional configurations that might be useful, such as filtering out specific targets from the coverage report.
🏁 Script executed:
#!/bin/bash # Display available configuration options for xcresulttool curl -s https://raw.githubusercontent.com/slidoapp/xcresulttool/v3.1.0/action.yml | grep -A 2 " [a-z]"Length of output: 836
Action Required: Review Additional xcresulttool Configuration Options
The current configuration in your workflow:
show-passed-tests: false if: success() || failure()is functionally acceptable. However, our verification of the xcresulttool action (v3.1.0) confirms it supports several configurable options beyond
show-passed-tests, including:
show-code-coverage(default: true)upload-bundles(default: always)- Other parameters such as
path,token, andtitleWhile there isn't a direct configuration key for filtering out specific targets from the coverage report, these additional options allow you to further customize the output and behavior of the action.
Recommendation:
Review if adjusting options like
show-code-coverageorupload-bundlescan help tailor the test result reporting to your needs.Use the command below to inspect the available configuration settings in the xcresulttool action if needed:
curl -s https://raw.githubusercontent.com/slidoapp/xcresulttool/v3.1.0/action.yml | grep -A 2 " [a-z]"Please verify if incorporating these adjustments improves clarity or filtering in your results configuration.
Sources/BuildTool/BuildTool.swift (2)
86-87: Consider making platform configurableThe platform is currently hardcoded to macOS. Consider making it an optional parameter like in other commands, in case there's a need to generate coverage for other platforms in the future.
- mutating func run() async throws { - let platform = Platform.macOS - let destinationSpecifier = try await platform.resolve() + @Option(help: "Platform to run tests on for code coverage.", default: Platform.macOS) + var platform: Platform + + mutating func run() async throws { + let destinationSpecifier = try await platform.resolve()
94-94: Consider making test plan configurableThe test plan is currently hardcoded as "UnitTests". Consider making it an optional parameter like resultBundlePath, which would provide more flexibility if additional test plans are added in the future.
@Option(help: "Pathname of where to output the .xcresult bundle.") var resultBundlePath: String + @Option(help: "The test plan to use for code coverage.", default: "UnitTests") + var testPlan: String + mutating func run() async throws { let platform = Platform.macOS let destinationSpecifier = try await platform.resolve() let scheme = "AblyChat" try await XcodeRunner.runXcodebuild( action: "test", scheme: scheme, destination: destinationSpecifier, - testPlan: "UnitTests", + testPlan: testPlan, resultBundlePath: resultBundlePath )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/check.yaml(2 hunks)Sources/BuildTool/BuildTool.swift(2 hunks)Sources/BuildTool/XcodeRunner.swift(2 hunks)
🔇 Additional comments (7)
.github/workflows/check.yaml (3)
141-174: Well-structured job for code coverage generationThe new
code-coveragejob is well-structured and follows the pattern of other jobs in the workflow. It correctly checks out the code, sets up Xcode, and generates the coverage report using both the BuildTool command and the xcresulttool action.
157-175: Great documentation of potential improvementsYour comments clearly document the current limitations and potential areas for improvement in the code coverage reporting, which is helpful for future development.
285-285: Correctly includes the new job in the all-checks-completed dependenciesThe code-coverage job is properly added to the dependencies of the all-checks-completed job, ensuring it's included in the required checks.
Sources/BuildTool/XcodeRunner.swift (2)
5-5: Function signature updated with backward-compatible parametersThe addition of optional parameters with default values ensures backward compatibility with existing code.
19-25: Clean implementation for adding conditional argumentsThe implementation correctly appends the new parameters to the xcodebuild arguments array only when they are provided.
Sources/BuildTool/BuildTool.swift (2)
19-19: Correctly adding new command to BuildTool configurationThe GenerateCodeCoverage command is properly added to the subcommands list, making it available as a CLI option.
75-98: Well-structured implementation of code coverage commandThe GenerateCodeCoverage struct follows the same pattern as other commands in this file and clearly implements the necessary functionality.
2 participants
Resolves #240.
Example of report (see comment in workflow YAML for some things that could be improved here):
Summary by CodeRabbit