Conversation
3861845 to
f06e6a0
Compare
Aryex
changed the title
0b01470 to
fdd43de
Compare
Signed-off-by: Alex Le <alex.le@improving.com>
Signed-off-by: Alex Le <alex.le@improving.com>
fdd43de to
7c45da7
Compare
currantw
left a comment
currantw
left a comment
There was a problem hiding this comment.
A few comments for suggestions. Looks good! ✅
| # We aims for a minimum of 80% coverage. However, our coverage at this time is much lower than this. | ||
| # Improvement is tracked in https://github.com/valkey-io/valkey-glide-ruby/issues/307 |
There was a problem hiding this comment.
In C#, I implemented a "rachet" mechanism for code coverage (see #384).
Basically, when coverage runs as part of CI, it fails if the measured coverage is less than these thresholds – and it fails if the measured coverage is more than these thresholds. To fix this, the user needs to increase these minimum coverage thresholds (e.g. from 40% to 41%) to match the actual measured coverage. That ensures that the coverage should only ever go up (or, at the very least, that a developer would need to explicitly decrease it when merging a PR, and so would theoretically have to justify why the coverage was going down).
Would it be possible to do something similar here?
This obviously isn't a silver bullet, but it should lead to test coverage gradually ticking up over time, even without dedicated efforts to improve it. Since I merged that "rachet" mechanism back in June, line coverage has increased from 70.6% to 76.5% and branch coverage from 50.9% to 61.7%.
| - name: Upload coverage report | ||
| if: always() | ||
| continue-on-error: true | ||
| uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 | ||
| with: | ||
| name: coverage-report-ruby-${{ matrix.version }}-${{ matrix.engine.type }}-${{ matrix.engine.version }}-${{ matrix.host.NAMED_OS }}-${{ matrix.host.ARCH }} | ||
| path: coverage/ | ||
| retention-days: 14 | ||
| if-no-files-found: ignore |
There was a problem hiding this comment.
Are you planning to use this? If there isn't a clear rationale for uploading this now, I think I would avoid it?
Signed-off-by: Alex Le <alex.le@improving.com>
Signed-off-by: Alex Le <alex.le@improving.com>
a0a846e to
2a975fc
Compare
Signed-off-by: Alex Le <alex.le@improving.com>
Signed-off-by: Alex Le <alex.le@improving.com>
Signed-off-by: Alex Le <alex.le@improving.com>
Signed-off-by: Alex Le <alex.le@improving.com>
Signed-off-by: Alex Le <alex.le@improving.com>
Signed-off-by: Alex Le <alex.le@improving.com>
3 participants
Summary
This PR add SimpleCov as the line coverage tooling.
Related
closes #288
Follow-up issue #307