FR-5793 - Rework tests #161
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #161 +/- ##
==========================================
+ Coverage 90.07% 90.38% +0.31%
==========================================
Files 13 13
Lines 2629 3402 +773
==========================================
+ Hits 2368 3075 +707
- Misses 229 295 +66
Partials 32 32
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
This is good. I really like the NewBasicChroot() bits, and the attention to detail regarding cleanup afterwards. The short/long test split is a good one as well. I have a question regarding that, but that's more of a curiosity - since the short tests execute so fast that it doesn't actually matter that much.
| err := os.Mkdir(workDir, 0755) | ||
| asserter.AssertErrNil(err, true) | ||
| defer os.RemoveAll(workDir) | ||
| t.Parallel() |
There was a problem hiding this comment.
This is a nice feature! From what I recall, the Python regular unittest module couldn't run tests in parallel, for instance. So that's a win.
There was a problem hiding this comment.
In fact by default Go will run tests in parallel. But only tests from different packages. This option is used to tell that in the same package, some tests can be run in parallel with others of the same package. This is nice in our case because we do not have a lot of packages and most of our tests are gathered in a single one (statemachine).
|
|
||
| - name: short tests | ||
| if: ${{ matrix.test-scenario == 'short' }} | ||
| run: sudo go test -timeout 0 -v -test.short -coverprofile=.coverage/coverage-short.out -covermode=atomic ./... |
There was a problem hiding this comment.
How does this work? Since I see some conditionals for when testing.Short() is on. But does this mean that the long tests execute all the long AND short tests? Or is there a way to just get the long ones run?
There was a problem hiding this comment.
Yes, the "long tests" are also executing the short ones. Unfortunately there is no practical way to exclude short ones. But since short ones takes less than 10s this is negligible comparing to the long ones. Moreover, since I upload the coverage in both cases, this is nice that long tests also cover the short ones so the final coverage is the complete one (and not only the coverage of long tests).
- skip long (time > ~20s) tests in short mode - get rid of useless subtests wrapping - replace defer by t.Cleanup - rename some test cases - rename saveCWD as restoreCWD - remove useless comments
- Keep on replacing defer with t.Cleanup - Fix some more test to run smoothly in parallel - ignore some slow tests in short mode
With 29 excluded tests, we can reach 83.9% coverage in around 7s, with parallelism
Next steps will be to: