Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

TestPollDeploymentRolloutStatus is unstable #2744

Closed
dgageot opened this issue Aug 28, 2019 · 3 comments · Fixed by #2896
Closed

TestPollDeploymentRolloutStatus is unstable #2744

dgageot opened this issue Aug 28, 2019 · 3 comments · Fixed by #2896
Assignees
@tejal29
Labels
kind/todo implementation task/epic for the skaffold team meta/test-flake priority/p0 Highest priority. We are actively looking at delivering it.

Comments

@dgageot
Copy link
Contributor

dgageot commented Aug 28, 2019

See https://travis-ci.com/GoogleContainerTools/skaffold/jobs/228963982#L166-L171

--- FAIL: TestPollDeploymentRolloutStatus (0.19s)
    cmd_helper.go:134: unable to run RunCmdOut() with command "kubectl --context kubecontext --namespace test rollout status deployment dep --watch=false"
    --- FAIL: TestPollDeploymentRolloutStatus/rollout_returns_did_not_stabilize_within_the_given_timeout (0.10s)
        testing.go:820: test executed panic(nil) or runtime.Goexit: subtest may have called FailNow on a parent test
FAIL
@balopat balopat added kind/todo implementation task/epic for the skaffold team priority/p0 Highest priority. We are actively looking at delivering it. labels Aug 28, 2019
@dgageot
Copy link
Contributor Author

dgageot commented Aug 29, 2019

The test is unstable and the way fake commands are created with the parent t instead of the child testing.T creates this funky error message.

dgageot added a commit to dgageot/skaffold that referenced this issue Sep 2, 2019
@dgageot
Improve fake commands handling
a7c14be 
 + avoid cases where the parent test would be mark
in error instead of the child test. See GoogleContainerTools#2744
 + simplify usage
 + make sure every usage look similar

Signed-off-by: David Gageot <[email protected]>
@dgageot dgageot mentioned this issue Sep 2, 2019
Merged
@balopat
Copy link
Contributor

balopat commented Sep 3, 2019

This is in progress, on her branch in #2750 this is going to be not a time based logic.

@tejal29 tejal29 mentioned this issue Sep 17, 2019
Merged
8 tasks
@tejal29
Copy link
Contributor

tejal29 commented Sep 18, 2019

This test does not check for number of calls any more. It also is not time dependent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tejal29 tejal29

Labels
kind/todo implementation task/epic for the skaffold team meta/test-flake priority/p0 Highest priority. We are actively looking at delivering it.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[Refactor] Move pollDeploymentStatus to resource.Deployment.CheckStatus tejal29/skaffold
3 participants
@dgageot @balopat @tejal29