fix: Correct test coverage script #1347
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
12 tasks
Codecov Report
@@ Coverage Diff @@
## master #1347 +/- ##
===========================================
- Coverage 77.88% 42.79% -35.10%
===========================================
Files 6 257 +251
Lines 529 15192 +14663
===========================================
+ Hits 412 6501 +6089
- Misses 112 7999 +7887
- Partials 5 692 +687 |
troian
requested changes
Aug 13, 2021
make/test-integration.mk
Outdated
| @@ -48,7 +48,7 @@ test-full: | |||
| test-coverage: | |||
| $(GO) test -tags=$(BUILD_MAINNET) -coverprofile=coverage.txt \ | |||
| -covermode=count \ | |||
| -coverpkg=$(COVER_PACKAGES) | |||
| -coverpkg=./... $(COVER_PACKAGES) | |||
There was a problem hiding this comment.
i think that needs more significant rework. there used to be incompatibility of coverprofile when coverpkg has multiple package. may need to go with loop here
There was a problem hiding this comment.
i think it should look like that
space := $(subst ,, )
comma := ,
test-coverage:
$(GO) test -tags=$(BUILD_MAINNET) -coverprofile=coverage.txt \
-covermode=count \
-coverpkg=$(subst $(space),$(comma),$(COVER_PACKAGES)) \
./...There was a problem hiding this comment.
Did something similar, now the script expands to:
GO111MODULE=on go test -tags= -coverprofile=coverage.txt \
-covermode=count \
-coverpkg=github.com/ovrclk/akash/app,github.com/ovrclk/akash/app/migrations,github.com/ovrclk/akash/app/params,github.com/ovrclk/akash/client,github.com/ovrclk/akash/client/broadcas
ter,github.com/ovrclk/akash/client/docs/statik,github.com/ovrclk/akash/cmd/akash,github.com/ovrclk/akash/cmd/akash/cmd,github.com/ovrclk/akash/cmd/common,github.com/ovrclk/akash/deploy/cmd,g
ithub.com/ovrclk/akash/docgen,github.com/ovrclk/akash/events,github.com/ovrclk/akash/events/cmd,github.com/ovrclk/akash/integration,github.com/ovrclk/akash/manifest,github.com/ovrclk/akash/p
kg/apis/akash.network/v1,github.com/ovrclk/akash/pkg/client/clientset/versioned,github.com/ovrclk/akash/pkg/client/clientset/versioned/fake,github.com/ovrclk/akash/pkg/client/clientset/versi
oned/scheme,github.com/ovrclk/akash/pkg/client/clientset/versioned/typed/akash.network/v1,github.com/ovrclk/akash/pkg/client/clientset/versioned/typed/akash.network/v1/fake,github.com/ovrclk
/akash/pkg/client/informers/externalversions,github.com/ovrclk/akash/pkg/client/informers/externalversions/akash.network,github.com/ovrclk/akash/pkg/client/informers/externalversions/akash.n
etwork/v1,github.com/ovrclk/akash/pkg/client/informers/externalversions/internalinterfaces,github.com/ovrclk/akash/pkg/client/listers/akash.network/v1,github.com/ovrclk/akash/provider,github
.com/ovrclk/akash/provider/bidengine,github.com/ovrclk/akash/provider/cluster,github.com/ovrclk/akash/provider/cluster/kube,github.com/ovrclk/akash/provider/cluster/types,github.com/ovrclk/a
kash/provider/cluster/util,github.com/ovrclk/akash/provider/cmd,github.com/ovrclk/akash/provider/event,github.com/ovrclk/akash/provider/gateway/rest,github.com/ovrclk/akash/provider/gateway/
utils,github.com/ovrclk/akash/provider/manifest,github.com/ovrclk/akash/provider/session,github.com/ovrclk/akash/provider/testutil,github.com/ovrclk/akash/pubsub,github.com/ovrclk/akash/sdku
til,github.com/ovrclk/akash/sdl,github.com/ovrclk/akash/testutil,github.com/ovrclk/akash/testutil/cli,github.com/ovrclk/akash/testutil/state,github.com/ovrclk/akash/types,github.com/ovrclk/a
kash/types/unit,github.com/ovrclk/akash/util/ctxlog,github.com/ovrclk/akash/util/metrics,github.com/ovrclk/akash/util/runner,github.com/ovrclk/akash/util/validation,github.com/ovrclk/akash/u
til/wsutil,github.com/ovrclk/akash/validation,github.com/ovrclk/akash/validation/constants,github.com/ovrclk/akash/x/audit,github.com/ovrclk/akash/x/audit/client/cli,github.com/ovrclk/akash/
x/audit/client/rest,github.com/ovrclk/akash/x/audit/handler,github.com/ovrclk/akash/x/audit/keeper,github.com/ovrclk/akash/x/audit/query,github.com/ovrclk/akash/x/audit/types,github.com/ovrc
lk/akash/x/cert,github.com/ovrclk/akash/x/cert/client/cli,github.com/ovrclk/akash/x/cert/handler,github.com/ovrclk/akash/x/cert/keeper,github.com/ovrclk/akash/x/cert/simulation,github.com/ov
rclk/akash/x/cert/types,github.com/ovrclk/akash/x/cert/utils,github.com/ovrclk/akash/x/deployment,github.com/ovrclk/akash/x/deployment/client/cli,github.com/ovrclk/akash/x/deployment/client/
rest,github.com/ovrclk/akash/x/deployment/handler,github.com/ovrclk/akash/x/deployment/keeper,github.com/ovrclk/akash/x/deployment/query,github.com/ovrclk/akash/x/deployment/simulation,githu
b.com/ovrclk/akash/x/deployment/types,github.com/ovrclk/akash/x/escrow,github.com/ovrclk/akash/x/escrow/client/cli,github.com/ovrclk/akash/x/escrow/client/rest,github.com/ovrclk/akash/x/escr
ow/keeper,github.com/ovrclk/akash/x/escrow/query,github.com/ovrclk/akash/x/escrow/types,github.com/ovrclk/akash/x/market,github.com/ovrclk/akash/x/market/client/cli,github.com/ovrclk/akash/x
/market/client/rest,github.com/ovrclk/akash/x/market/handler,github.com/ovrclk/akash/x/market/hooks,github.com/ovrclk/akash/x/market/keeper,github.com/ovrclk/akash/x/market/query,github.com/
ovrclk/akash/x/market/simulation,github.com/ovrclk/akash/x/market/types,github.com/ovrclk/akash/x/provider,github.com/ovrclk/akash/x/provider/client/cli,github.com/ovrclk/akash/x/provider/cl
ient/rest,github.com/ovrclk/akash/x/provider/config,github.com/ovrclk/akash/x/provider/handler,github.com/ovrclk/akash/x/provider/keeper,github.com/ovrclk/akash/x/provider/query,github.com/o
vrclk/akash/x/provider/simulation,github.com/ovrclk/akash/x/provider/types \
./...
troian
reviewed
Aug 13, 2021
make/test-integration.mk
Outdated
| @@ -1,4 +1,4 @@ | |||
| COVER_PACKAGES = $(shell go list ./... | grep -v mock) | |||
| COVER_PACKAGES = $(shell go list ./... | grep -v mock | tr "\n" "," | rev | cut -c2- | rev) | |||
There was a problem hiding this comment.
maybe just bash substitution then?
There was a problem hiding this comment.
was having issues making bash substitution work with makefile, so just used the makefile way as initially suggested by you:
space := $(subst ,, ) comma := , test-coverage: $(GO) test -tags=$(BUILD_MAINNET) -coverprofile=coverage.txt \ -covermode=count \ -coverpkg=$(subst $(space),$(comma),$(COVER_PACKAGES)) \ ./...
troian
approved these changes
Aug 17, 2021
boz
reviewed
Aug 17, 2021
make/test-integration.mk
Outdated
| COVER_PACKAGES = $(shell go list ./... | grep -v mock) | ||
| space = $(subst ,, ) | ||
| comma = , | ||
| COVER_PACKAGES = $(subst $(space),$(comma),$(shell go list ./... | grep -v mock)) |
There was a problem hiding this comment.
I'd suggest doing it in shell instead of the make hack:
$(shell go list ./... | grep -v mock | tr ' ' ',')
I think that we should exclude proto-generated things if we can, but we can iterate on this later.
arijitAD
force-pushed
the
fix-coverage
branch
2 times, most recently
from
c424493 to
f40af0e
Compare
boz
approved these changes
Aug 18, 2021
There was a problem hiding this comment.
Sorry, I didn't realize that this was multi-line. If you need a trailing comma then here are some non-xargs options. xargs will have a limit to the number of arguments that can be passed.
COVER_PACKAGES = $(shell go list ./... | grep -v mock | tr '\n' ',')COVER_PACKAGES = $(shell go list ./... | grep -v mock | paste -sd, ),Go ahead and merge whichever you'd like, tho.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
The coverage script was broken till now. Previously, this script was expanding to following command:
This resulted in only the first package in the list, i.e.,
github.com/ovrclk/akash/appbeing used as-coverpkg, while rest of the packages after that were arguments to thego testcommand. So, it was running tests in all of the remaining packages and was using them to measure the coverage of onlygithub.com/ovrclk/akash/app, resulting in the coverage being reported too high while it was actually something else overall.Motivation and Context
design/approvedlabelHow Has This Been Tested?
Types of changes
Checklist:
git commit -s