Skip to content

Prevent potential panic in TestLogsAPIUntil#35808

Merged
dnephin merged 1 commit intomoby:masterfrom
thaJeztah:prevent-panic-in-test
Dec 19, 2017
Merged

Prevent potential panic in TestLogsAPIUntil#35808
dnephin merged 1 commit intomoby:masterfrom
thaJeztah:prevent-panic-in-test

Conversation

@thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Dec 15, 2017 •
edited
Loading

Seen failing on Windows (#35793 (comment)):

22:27:52 ----------------------------------------------------------------------
22:27:52 PANIC: docker_api_logs_test.go:152: DockerSuite.TestLogsAPIUntil
22:27:52
22:27:52 ... Panic: runtime error: index out of range (PC=0x45AC01)
22:27:52
22:27:52 d:/CI/CI-7caa30e89/go/src/runtime/asm_amd64.s:509
22:27:52   in call32
22:27:52 d:/CI/CI-7caa30e89/go/src/runtime/panic.go:491
22:27:52   in gopanic
22:27:52 d:/CI/CI-7caa30e89/go/src/runtime/panic.go:28
22:27:52   in panicindex
22:27:52 docker_api_logs_test.go:175
22:27:52   in DockerSuite.TestLogsAPIUntil
22:27:52 d:/CI/CI-7caa30e89/go/src/runtime/asm_amd64.s:509
22:27:52   in call32
22:27:52 d:/CI/CI-7caa30e89/go/src/reflect/value.go:434
22:27:52   in Value.call
22:27:52 d:/CI/CI-7caa30e89/go/src/reflect/value.go:302
22:27:52   in Value.Call
22:27:52 c:/gopath/src/github.com/docker/docker/vendor/github.com/go-check/check/check.go:816
22:27:52   in suiteRunner.forkTest.func1
22:27:52 c:/gopath/src/github.com/docker/docker/vendor/github.com/go-check/check/check.go:672
22:27:52   in suiteRunner.forkCall.func1
22:27:52 d:/CI/CI-7caa30e89/go/src/runtime/asm_amd64.s:2337
22:27:52   in goexit
22:27:54
22:27:54 ----------------------------------------------------------------------

- What I did

- How I did it

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

@thaJeztah
Prevent potential panic in TestLogsAPIUntil
e77de78 
Seen failing on Windows:

    22:27:52 ----------------------------------------------------------------------
    22:27:52 PANIC: docker_api_logs_test.go:152: DockerSuite.TestLogsAPIUntil
    22:27:52
    22:27:52 ... Panic: runtime error: index out of range (PC=0x45AC01)
    22:27:52
    22:27:52 d:/CI/CI-7caa30e89/go/src/runtime/asm_amd64.s:509
    22:27:52   in call32
    22:27:52 d:/CI/CI-7caa30e89/go/src/runtime/panic.go:491
    22:27:52   in gopanic
    22:27:52 d:/CI/CI-7caa30e89/go/src/runtime/panic.go:28
    22:27:52   in panicindex
    22:27:52 docker_api_logs_test.go:175
    22:27:52   in DockerSuite.TestLogsAPIUntil
    22:27:52 d:/CI/CI-7caa30e89/go/src/runtime/asm_amd64.s:509
    22:27:52   in call32
    22:27:52 d:/CI/CI-7caa30e89/go/src/reflect/value.go:434
    22:27:52   in Value.call
    22:27:52 d:/CI/CI-7caa30e89/go/src/reflect/value.go:302
    22:27:52   in Value.Call
    22:27:52 c:/gopath/src/github.com/docker/docker/vendor/github.com/go-check/check/check.go:816
    22:27:52   in suiteRunner.forkTest.func1
    22:27:52 c:/gopath/src/github.com/docker/docker/vendor/github.com/go-check/check/check.go:672
    22:27:52   in suiteRunner.forkCall.func1
    22:27:52 d:/CI/CI-7caa30e89/go/src/runtime/asm_amd64.s:2337
    22:27:52   in goexit
    22:27:54
    22:27:54 ----------------------------------------------------------------------

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah
thaJeztah commented Dec 15, 2017
View reviewed changes
integration-cli/docker_api_logs_test.go

// Get timestamp of second log line
allLogs := extractBody(c, types.ContainerLogsOptions{Timestamps: true, ShowStdout: true})
c.Assert(len(allLogs), checker.GreaterOrEqualThan, 3)
Copy link
Member Author

@thaJeztah thaJeztah Dec 15, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the logs return an empty "" as last entry, so switched to checker.GreaterOrEqualThan instead of checker.Equal

@arm64b
Copy link
Contributor

arm64b commented Dec 15, 2017

HI @thaJeztah , how can I rebuild a failed CI test, do I need some kind of access right to do that❓
Happy new year :)

@thaJeztah
Copy link
Member Author

thaJeztah commented Dec 15, 2017

I can restart them for you 😄

z node seems to be having issues 😕

hm, flaky test? https://jenkins.dockerproject.org/job/Docker-PRs-experimental/38413/console

08:37:09 ----------------------------------------------------------------------
08:37:09 FAIL: docker_api_swarm_service_test.go:614: DockerSwarmSuite.TestAPISwarmServicesPlugin
08:37:09 
08:37:09 [df078edbf8ffe] waiting for daemon to start
08:37:09 [df078edbf8ffe] daemon started
08:37:09 
08:37:09 [d23df75ab98fd] waiting for daemon to start
08:37:09 [d23df75ab98fd] daemon started
08:37:09 
08:37:09 [d76fdff58109a] waiting for daemon to start
08:37:09 [d76fdff58109a] daemon started
08:37:09 
08:37:09 docker_api_swarm_service_test.go:683:
08:37:09     waitAndAssert(c, defaultReconciliationTimeout, d1.CheckPluginRunning(repo), checker.False)
08:37:09 docker_utils_test.go:465:
08:37:09     c.Assert(v, checker, args...)
08:37:09 ... obtained bool = true
08:37:09 ... {Config:{Args:{Description: Name: Settable:[] Value:[]} Description: DockerVersion:dev Documentation: Entrypoint:[/basic] Env:[] Interface:{Socket:basic.sock Types:[.docker.dummy/1.0/]} IpcHost:false Linux:{AllowAllDevices:false Capabilities:[] Devices:[]} Mounts:[] Network:{Type:} PidHost:false PropagatedMount: User:{GID:0 UID:0} WorkDir: Rootfs:0xc4201f77d0} Enabled:true ID:4d796b8577074a6d8d42ccbdc0fcd24b2b699c5ea6d8df781d40ab37ccaba011 Name:127.0.0.1:5000/swarm/test:v1 PluginReference:127.0.0.1:5000/swarm/test:v1 Settings:{Args:[] Devices:[] Env:[] Mounts:[]}}
08:37:09 
08:37:09 [df078edbf8ffe] exiting daemon
08:37:09 [d23df75ab98fd] exiting daemon
08:37:09 [d76fdff58109a] exiting daemon
08:37:34 
08:37:34 ----------------------------------------------------------------------

@arm64b
Copy link
Contributor

arm64b commented Dec 15, 2017

Ah, so you a VIP can re-trigger the rebuild action, so please help me to rebuild the janky failure in PR #35772 😄. Thanks you 👍

@thaJeztah thaJeztah mentioned this pull request Dec 15, 2017
Merged
@thaJeztah
Copy link
Member Author

thaJeztah commented Dec 15, 2017

Done! 👍

@thaJeztah
Copy link
Member Author

thaJeztah commented Dec 18, 2017

Failure on z looks like #35825

vdemeester
vdemeester approved these changes Dec 19, 2017
View reviewed changes
Copy link
Member

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🐯
cc @dnephin

@dnephin dnephin merged commit 602bce1 into moby:master Dec 19, 2017
@thaJeztah thaJeztah deleted the prevent-panic-in-test branch December 19, 2017 16:29
@thaJeztah thaJeztah mentioned this pull request Feb 19, 2018
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@vdemeester vdemeester vdemeester approved these changes

@dnephin dnephin dnephin approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area/testing status/2-code-review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@thaJeztah @arm64b @vdemeester @dnephin @GordonTheTurtle