Integration: use testenv.APIClient()#38473
Conversation
c124fbf to
6309b7d
Compare
|
Looks like I need a nolint comment |
6309b7d to
b84a1e2
Compare
|
Interesting; Details |
|
Looks like |
Indeed… I remember discussing about that a bit (as what should we do to make it safe to use for parallel tests), but it's definitely not the case as of today 😅 That said, |
|
The problem is be triggered by It's a pity that |
`testEnv` is a package-level variable, so protecting / restoring `testEnv` in parallel will result in "concurrent map write" errors. This patch removes `t.Parallel()` from tests that use this functionality (through `defer setupTest(t)()`). Note that _subtests_ can still be run in parallel, as the defer will be called after all subtests have completed. Signed-off-by: Sebastiaan van Stijn <[email protected]>
A client is already created in testenv.New(), so we can just as well use that one, instead of creating a new client. Signed-off-by: Sebastiaan van Stijn <[email protected]>
b84a1e2 to
0de62d9
Compare
Codecov Report
@@ Coverage Diff @@
## master #38473 +/- ##
=========================================
Coverage ? 36.63%
=========================================
Files ? 608
Lines ? 45040
Branches ? 0
=========================================
Hits ? 16502
Misses ? 26256
Partials ? 2282 |
|
pushed a commit to remove some |
| func TestBuildWithRemoveAndForceRemove(t *testing.T) { | ||
| skip.If(t, testEnv.DaemonInfo.OSType == "windows", "FIXME") | ||
| defer setupTest(t)() | ||
| t.Parallel() |
There was a problem hiding this comment.
Subtests are still run in parallel on this test
|
|
||
| func TestMountDaemonRoot(t *testing.T) { | ||
| skip.If(t, testEnv.DaemonInfo.OSType == "windows" || testEnv.IsRemoteDaemon()) | ||
| t.Parallel() |
There was a problem hiding this comment.
Subtests are still run in parallel on this test
| skip.If(t, testEnv.OSType == "windows", "TODO enable on windows") | ||
|
|
||
| client := request.NewAPIClient(t) | ||
| defer setupTest(t)() |
There was a problem hiding this comment.
Wondering if we should spin up a new daemon for this one; I'll check how long this test takes to run
There was a problem hiding this comment.
13:32:12 === RUN TestImportExtremelyLargeImageWorks
13:34:14 --- PASS: TestImportExtremelyLargeImageWorks (122.92s)
Looks like that may be worth considering 👍
|
s390x failure is unrelated; I'll merge |
A client is already created in
testenv.New(), so we can just as well use that one, instead of creating a new client.