-
Notifications
You must be signed in to change notification settings - Fork 16
fix: matrix not run properly #195
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
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #195 +/- ##
===========================================
- Coverage 93.04% 82.06% -10.98%
===========================================
Files 16 18 +2
Lines 1596 1974 +378
===========================================
+ Hits 1485 1620 +135
- Misses 83 314 +231
- Partials 28 40 +12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
I think the reason for the difference in test coverage is the change to line 107 and to line 109 in .circleci/config.yml which is now also testing examples and adding them to the report. These were not tested in the past.
Compare...
PR 195 (this PR)
https://output.circle-artifacts.com/output/job/6b174d2b-d7e3-40cc-ac1a-fb0320d5ba7a/artifacts/0/tmp/artifacts/coverage.html#file0
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 last review comment
3d67e28 to
29b2a58
Compare
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.
I see that batching tests are missing from the coverage report. My suggestion yesterday was inexact.
.circleci/config.yml
Outdated
| command: | | ||
| if [[ "$CIRCLE_BRANCH" == pull/* ]]; then | ||
| GOEXPERIMENT=nocoverageredesign go test -v -cover -coverprofile=coverage.out ./... | ||
| go test -coverprofile=coverage.out ./influxdb3/ |
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.
Sorry, my suggestion yesterday was inexact. I notice now that tests in influxdb3/batching are not being run. I think the correct command is most likely.
go test -v -cover -coverprofile=coverage.out ./influxdb3/...
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.
For some reason coverage is still down 11%. Haven't time right now to experiment further.

Closes #
Proposed Changes
Checklist