Remove some checkers and use gotest.tools#39001
Conversation
thaJeztah
force-pushed
the
reduce_checkers
branch
2 times, most recently
from
9ccd4cb to
94601f7
Compare
thaJeztah
force-pushed
the
reduce_checkers
branch
from
94601f7 to
3a14c53
Compare
Codecov Report
@@ Coverage Diff @@
## master #39001 +/- ##
==========================================
+ Coverage 36.89% 36.91% +0.01%
==========================================
Files 613 613
Lines 45421 45421
==========================================
+ Hits 16759 16766 +7
+ Misses 26370 26365 -5
+ Partials 2292 2290 -2 |
thaJeztah
force-pushed
the
reduce_checkers
branch
11 times, most recently
from
48404cf to
b279065
Compare
thaJeztah
changed the title
thaJeztah
force-pushed
the
reduce_checkers
branch
9 times, most recently
from
678a266 to
55895d4
Compare
thaJeztah
changed the title
|
ping @vdemeester @yongtang @cpuguy83 PTAL |
thaJeztah
force-pushed
the
reduce_checkers
branch
from
55895d4 to
184abf4
Compare
Signed-off-by: Sebastiaan van Stijn <[email protected]>
There was a problem hiding this comment.
Interesting; looks like this was incorrect on multiple levels;
erris actually anexitCode(int), so checking fornil/not-nildoesn't really make sense- I rewrote it to check for a non-zero exit code (as that's what the code above appeared to be checking for), but it actually should run succesfully. The test passed, because
0 != nil🤷♂️
I'll fix my rewrite, as it's currently failing because of the above;
02:53:53 FAIL: docker_cli_restart_test.go:80: DockerSuite.TestRestartDisconnectedContainer
02:53:53
02:53:53 assertion failed: exitCode is 0
184abf4 to
57e8e0f
Compare
Signed-off-by: Sebastiaan van Stijn <[email protected]>
thaJeztah
force-pushed
the
reduce_checkers
branch
from
57e8e0f to
6345208
Compare
|
whoop, green! @vdemeester ptal 🤗🙈🙊🙉 |
vdemeester
left a comment
vdemeester
left a comment
There was a problem hiding this comment.
LGTM 🐯
A comment though : we should document why we are using gotest.tools assert instead of go-check functions. Also, we may want to switch to testing.T at some point 💃
| TimeEquals = shakers.TimeEquals | ||
| True = shakers.True | ||
| TimeIgnore = shakers.TimeIgnore | ||
| Contains = shakers.Contains |
There was a problem hiding this comment.
Can we get rid of all of those ?
There was a problem hiding this comment.
Yes, we should; some of these were used as part of a waitForSomething loops, so I had to look into those (didn't come round to that yet); thought to at least get some of this stuff unused, to (to an extent) help migrating these tests to the new suite.
Still a lot left to clean up though 😅
|
Thanks! |
4 participants
found some WIP changes in my stash, so let me push it as a PR 😅