-
Notifications
You must be signed in to change notification settings - Fork 0
ci: Add codecov test results #24
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
Conversation
WalkthroughThe changes introduce JUnit XML test logging for both unit and functional tests by adding the JUnitXml.TestLogger package and updating test commands and CI workflow steps. Package versions for test-related dependencies are incremented, and new steps are added to upload test results to Codecov. The .NET SDK version is also updated. Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant GitHub Actions
participant dotnet test
participant Codecov
Developer->>GitHub Actions: Push code / PR
GitHub Actions->>dotnet test: Run unit & functional tests with JUnit logger
dotnet test-->>GitHub Actions: Output results.xml files
GitHub Actions->>Codecov: Upload JUnit XML test results (unit & functional)
GitHub Actions->>Codecov: Upload coverage reports
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (7)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #24 +/- ##
=======================================
Coverage 93.76% 93.76%
=======================================
Files 27 27
Lines 1027 1027
Branches 151 151
=======================================
Hits 963 963
Misses 32 32
Partials 32 32 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (4)
src/CompositeKey.SourceGeneration.UnitTests/CompositeKey.SourceGeneration.UnitTests.csproj (1)
21-27: AddPrivateAssetsfor the test logger for cleanlinessAlthough the project is non-packable, it’s still a good habit to mark tooling-only packages as private to prevent unintended transitive flow if that flag is flipped later.
- <PackageReference Include="JUnitXml.TestLogger" /> + <PackageReference Include="JUnitXml.TestLogger"> + <PrivateAssets>all</PrivateAssets> + <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets> + </PackageReference>src/CompositeKey.SourceGeneration.FunctionalTests/CompositeKey.SourceGeneration.FunctionalTests.csproj (1)
18-26: Mirror thePrivateAssetspattern here as wellSame rationale as for the unit tests csproj – keeps the dependency surface tidy.
- <PackageReference Include="JUnitXml.TestLogger" /> + <PackageReference Include="JUnitXml.TestLogger"> + <PrivateAssets>all</PrivateAssets> + <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets> + </PackageReference>.github/workflows/ci.yml (1)
84-95: Token is already passed via ENV in previous Codecov stepYou supply
token: ${{ secrets.CODECOV_TOKEN }}here, but the officialcodecov/test-results-actionautomatically picks upCODECOV_TOKENfrom the environment (same as the coverage upload step).
The explicit parameter is redundant and can be removed to avoid accidental token-echoing in the logs.src/CompositeKey.SourceGeneration.FunctionalTests/packages.lock.json (1)
27-32: Package ID casingNuGet IDs are case-insensitive, but everywhere else in the repo the package is referenced as
JUnitXml.TestLogger(upper-case U). Keeping the canonical casing improves readability and avoids churn in future automated updates.Also applies to: 115-118
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/ci.yml(2 hunks)global.json(1 hunks)src/CompositeKey.SourceGeneration.FunctionalTests/CompositeKey.SourceGeneration.FunctionalTests.csproj(1 hunks)src/CompositeKey.SourceGeneration.FunctionalTests/packages.lock.json(8 hunks)src/CompositeKey.SourceGeneration.UnitTests/CompositeKey.SourceGeneration.UnitTests.csproj(1 hunks)src/CompositeKey.SourceGeneration.UnitTests/packages.lock.json(8 hunks)src/Directory.Packages.props(1 hunks)
🔇 Additional comments (4)
global.json (1)
3-3: Confirm the CI runner has the matching .NET SDK 9.0.200Bumping
global.jsonto 9.0.200 is fine, but make sure the image used in GitHub Actions (or any other CI) explicitly installs that exact SDK. A version drift between local builds and CI is a common flake source.src/Directory.Packages.props (1)
24-30: LGTM – centralised version bump looks good
JunitXml.TestLoggeraddition and the minor test-stack bumps are consistent and safe. No issues spotted.src/CompositeKey.SourceGeneration.UnitTests/packages.lock.json (1)
17-22: Auto-generated lockfile – looks in syncThe new entry for
JunitXml.TestLoggerand the incremental version bumps match the central props file. No manual action required.src/CompositeKey.SourceGeneration.FunctionalTests/packages.lock.json (1)
35-41: Ensure SDK/Test-host version bump is consistent across all projects
Microsoft.NET.Test.Sdkwas bumped to17.14.1here. Please verify that every test project and the centralDirectory.Packages.propsreference the same version to prevent CI restore conflicts.Also applies to: 1152-1159
ef53d16 to
53157a8
Compare
Summary by CodeRabbit