test: Remove CLI tests from make test #1643
Conversation
AndrewSisley
added
code quality
AndrewSisley
changed the title
Codecov ReportPatch coverage has no change and project coverage change:
@@ Coverage Diff @@
## develop #1643 +/- ##
===========================================
- Coverage 75.64% 75.53% -0.11%
===========================================
Files 200 200
Lines 20807 20807
===========================================
- Hits 15738 15715 -23
- Misses 3992 4008 +16
- Partials 1077 1084 +7
Flags with carried forward coverage won't be shown. Click here to find out more. see 5 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Makefile
Outdated
| .PHONY: test\:short | ||
| test\:short: | ||
| go test $(DEFAULT_TEST_DIRECTORIES) |
There was a problem hiding this comment.
thought: Possible conflict with the make:quick rule:
.PHONY: test\:quick
test\:quick:
gotestsum --format pkgname -- $(DEFAULT_TEST_DIRECTORIES)
introduced in my #1609 which will merge shortly
There was a problem hiding this comment.
You can just rename this to test:quick-go
There was a problem hiding this comment.
I'll check how much overhead gotestsum adds and then maybe drop this
There was a problem hiding this comment.
Diff seems insignificant :) Dropping this.
- drop
| @@ -226,6 +232,10 @@ test\:lens: | |||
| @$(MAKE) deps:lens | |||
| gotestsum --format testname -- ./$(LENS_TEST_DIRECTORY)/... $(TEST_FLAGS) | |||
|
|
|||
| .PHONY: test\:cli | |||
| test\:cli: | |||
| gotestsum --format testname -- ./$(CLI_TEST_DIRECTORY)/... $(TEST_FLAGS) | |||
There was a problem hiding this comment.
question: For lens and cli I noticed we are also using testname over pkgname? Is that desired? I find testname to be useful in ci, but locally I prefer pgkname
There was a problem hiding this comment.
I just copy-pasted the settings from the existing commands we had. I actually don't really care for either of them and other than for a quick sanity check that it has actually run the tests, I would probably be happy with just the count and any failure logs.
There was a problem hiding this comment.
Because it is being used in the ci command can leave as is
They are slow and it is distruptive to have to wait for them when working on core stuff
AndrewSisley
force-pushed
the
1642-test-cli
branch
2 times, most recently
from
30493b5
to
d24f8e9
Compare
## Relevant issue(s) Resolves sourcenetwork#1642 ## Description Removes CLI tests from `make test` as they take ages to run and it is disruptive to dev-flow. Runtimes on my machine: Old `make test`: 72s New `make test`: 30s
Relevant issue(s)
Resolves #1642
Description
Removes CLI tests from
make testas they take ages to run and it is disruptive to dev-flow.Also adds
make test:shortwhich is even leaner, skipping all the extras like the race detector.Runtimes on my machine:
Old
make test: 72sNew
make test: 30sNew
make test:short: 21s