[tests] Cleanup and better logging#2222
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2222 +/- ##
==========================================
+ Coverage 60.27% 60.29% +0.01%
==========================================
Files 124 124
Lines 20149 20149
==========================================
+ Hits 12145 12149 +4
+ Misses 6639 6633 -6
- Partials 1365 1367 +2 |
…CA tests, similar to how we log the test names in the integration tests. Signed-off-by: Ying Li <ying.li@docker.com>
| rootCA, err := ca.NewRootCA(testutils.ECDSACertChain[2], nil, nil, ca.DefaultNodeCertExpiration, nil) | ||
| require.NoError(t, err) | ||
|
|
||
| ctx := log.WithLogger(context.Background(), log.L.WithFields(logrus.Fields{ |
There was a problem hiding this comment.
Is this supposed to be t.Context? I only ask because elsewhere this PR changes a lot of context.Background() into t.Context. Maybe it doesn't matter much for the logger?
There was a problem hiding this comment.
On all the other ones it was tc.Context - tc being cautil.go's TestCA object. This particular test doesn't create one of those, hence creating the context manually.
|
FWIW this LGTM, at least so far as I am able to judge (I'm not terribly familiar with this code). |
|
LGTM |
|
CI failed after the merge to master: Related? |
|
I've seen that a couple times on master prior to this. I haven't been able to replicate it locally reliably. |
This PR includes some test cleanups discovered while debugging #2221:
The goroutine that updates to the RootCA of the testing CA server never actually terminates after a test, because the context is never canceled. This makes sure that goroutine is cleaned up.
One of the fake CA servers is never stopped after a test - added a defer stop.
Similar to how the integration tests log the test names, do the same for the testing CA so we can tell which logs belong to which test.
cc @aaronlehmann @ijc25 This does not actually fix the stated issue above, but would be helpful for debugging and may reduce some resource consumption for running the tests.