-
Notifications
You must be signed in to change notification settings - Fork 1
Add comprehensive test coverage reporting infrastructure #33
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: jbrinkman <[email protected]>
02b0910
to
e6ba8b4
Compare
Signed-off-by: jbrinkman <[email protected]>
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.
Please update dev guide
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.
Wanted to have a discussion about taskfile usage as we have a lot of details in the developer guide that probably could move into the taskfile. Didn't want to do that until we discussed.
…rk parameter support- Remove separate test-coverage.yml workflow- Add conditional coverage steps to tests.yml workflow using coverage-check output- Add framework parameter support to Taskfile.yml- Coverage now runs only on Ubuntu x64 with .NET 8.0.x to optimize CI resources- Add .NET 9.0.x support to test workflows for C# language feature compatibility Signed-off-by: jbrinkman <[email protected]>
|
||
vars: | ||
TEST_RESULTS_DIR: testresults | ||
REPORTS_DIR: reports |
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.
Consider updating CI - it stores the report in the project root
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.
Not sure what you want here. All reports are getting uploaded as artifacts correctly. I don't see anything to fix here.
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.
See CI job lines 97-99
…orkflow Signed-off-by: jbrinkman <[email protected]>
Signed-off-by: jbrinkman <[email protected]>
Signed-off-by: jbrinkman <[email protected]>
Signed-off-by: jbrinkman <[email protected]>
…iggers Signed-off-by: jbrinkman <[email protected]>
…frameworks Signed-off-by: jbrinkman <[email protected]>
Taskfile.yml
Outdated
./install_and_test.sh -no-tls -minimal -only-glide -data 1 -tasks 10 -csharp -dotnet-framework net8.0 | ||
./install_and_test.sh -no-tls -minimal -only-glide -data 1 -tasks 10 -csharp -dotnet-framework net6.0 |
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.
./install_and_test.sh -no-tls -minimal -only-glide -data 1 -tasks 10 -csharp -dotnet-framework net8.0 | |
./install_and_test.sh -no-tls -minimal -only-glide -data 1 -tasks 10 -csharp -dotnet-framework net6.0 | |
./install_and_test.sh -no-tls -minimal -only-glide -data 1 -tasks 10 -csharp |
Probably this works too
…k validation Signed-off-by: jbrinkman <[email protected]>
@@ -109,6 +107,10 @@ jobs: | |||
redis-server --save "" --daemonize "yes" | |||
./install_and_test.sh -no-tls -minimal -only-glide -data 1 -tasks 10 -csharp -dotnet-framework net${{ matrix.dotnet }} | |||
|
|||
- name: Run coverage |
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.
This runs all tests second time. I propose to add a condition (like else) for test step (lines 97-99)
|
||
vars: | ||
TEST_RESULTS_DIR: testresults | ||
REPORTS_DIR: reports |
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.
See CI job lines 97-99
echo "Line Coverage: ${LINE_COVERAGE}%" | ||
echo "Branch Coverage: ${BRANCH_COVERAGE}%" | ||
else | ||
echo "No coverage summary found. Run 'task coverage:report' first." |
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.
echo "No coverage summary found. Run 'task coverage:report' first." | |
echo "No coverage summary found. Run 'task coverage:report' first." | |
exit 1 |
Co-authored-by: Yury-Fridlyand <[email protected]> Signed-off-by: Joseph Brinkman <[email protected]>
Co-authored-by: Yury-Fridlyand <[email protected]> Signed-off-by: Joseph Brinkman <[email protected]>
Summary
Implements comprehensive test coverage reporting infrastructure for the C# Valkey GLIDE client project as requested in issue #27. This PR adds automated coverage collection, report generation, and CI/CD integration.
Changes Made
🔧 Task-based Build System (
Taskfile.yml
)coverage
,coverage:unit
,coverage:integration
🚀 GitHub Actions Workflow (
.github/workflows/test-coverage.yml
)📝 Git Configuration (
.gitignore
)[Rr]eports/
to exclude generated coverage reports🎯 Features
Coverage Collection
Report Generation
CI/CD Integration
🔍 Technical Implementation
Coverage Pipeline
Quality Features
Valkey.Glide*
assemblies only📊 Acceptance Criteria ✅
🎉 Usage Examples
Local Development
CI/CD
Advanced Usage
🔗 Integration Ready
The coverage data is output in standard formats, making it ready for integration with:
Fixes #27