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.

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

crypto/tls: TestVerifyHostnameResumed consistently failing in longtest builder #32978

Closed
bcmills opened this issue Jul 8, 2019 · 4 comments
Closed

crypto/tls: TestVerifyHostnameResumed consistently failing in longtest builder #32978

bcmills opened this issue Jul 8, 2019 · 4 comments
Labels
FrozenDueToAge release-blocker Soon This needs to be done soon. (regressions, serious bugs, outages) Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone
Go1.13

Comments

@bcmills
Copy link
Contributor

bcmills commented Jul 8, 2019

crypto/tls.TestVerifyHostnameResumed is consistently failing in the linux-amd64-longtest builder.

First failure was at CL 184099, which appears to be unrelated. That suggests some sort of change in a non-hermetic dependency, or perhaps in the builder itself.

Example: https://build.golang.org/log/67d2b12bec6bf70eb818bb3246aee32990ecd9e6

--- FAIL: TestVerifyHostnameResumed (0.10s)
    --- FAIL: TestVerifyHostnameResumed/TLSv12 (0.05s)
        tls_test.go:383: Subsequent connection unexpectedly didn't resume
    --- FAIL: TestVerifyHostnameResumed/TLSv13 (0.05s)
        tls_test.go:383: Subsequent connection unexpectedly didn't resume
FAIL
FAIL	crypto/tls	0.828s

CC @agl
@bcmills bcmills added Testing An issue that has been verified to require only test changes, not just a test failure. release-blocker Soon This needs to be done soon. (regressions, serious bugs, outages) labels Jul 8, 2019
@bcmills bcmills added this to the Go1.13 milestone Jul 8, 2019
@dmitshur
Copy link
Contributor

dmitshur commented Jul 8, 2019

I can reproduce this on macOS with Go 1.12.6.

~ $ go version
go version go1.12.6 darwin/amd64
~ $ go test -count=1 crypto/tls
--- FAIL: TestVerifyHostnameResumed (1.62s)
    --- FAIL: TestVerifyHostnameResumed/TLSv13 (0.55s)
        tls_test.go:392: Subsequent connection unexpectedly didn't resume
FAIL
FAIL	crypto/tls	2.943s

@gopherbot
Copy link

Change https://golang.org/cl/186239 mentions this issue: crypto/tls: remove TestVerifyHostnameResumed

@gopherbot
Copy link

Change https://golang.org/cl/186277 mentions this issue: [release-branch.go1.12] crypto/tls: remove TestVerifyHostnameResumed

@gopherbot
Copy link

Change https://golang.org/cl/186279 mentions this issue: [release-branch.go1.11] crypto/tls: remove TestVerifyHostnameResumed

gopherbot pushed a commit that referenced this issue Jul 15, 2019
@FiloSottile
[release-branch.go1.12] crypto/tls: remove TestVerifyHostnameResumed
8759b53 
Session resumption is not a reliable TLS behavior: the server can decide
to reject a session ticket for a number of reasons, or no reason at all.
This makes this non-hermetic test extremely brittle.

It's currently broken on the builders for both TLS 1.2 and TLS 1.3, and
I could reproduce the issue for TLS 1.3 only. As I was debugging it, it
started passing entirely on my machine.

In practice, it doesn't get us any coverage as resumption is already
tested with the recorded exchange tests, and TestVerifyHostname still
provides a smoke test checking that we can in fact talk TLS.

Updates #32978

Change-Id: I63505e22ff7704f25ad700d46e4ff14850ba5d3c
Reviewed-on: https://go-review.googlesource.com/c/go/+/186239
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
(cherry-picked from 20e4540)
Reviewed-on: https://go-review.googlesource.com/c/go/+/186277
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
gopherbot pushed a commit that referenced this issue Jul 15, 2019
@FiloSottile
[release-branch.go1.11] crypto/tls: remove TestVerifyHostnameResumed
7a853b6 
Session resumption is not a reliable TLS behavior: the server can decide
to reject a session ticket for a number of reasons, or no reason at all.
This makes this non-hermetic test extremely brittle.

It's currently broken on the builders for both TLS 1.2 and TLS 1.3, and
I could reproduce the issue for TLS 1.3 only. As I was debugging it, it
started passing entirely on my machine.

In practice, it doesn't get us any coverage as resumption is already
tested with the recorded exchange tests, and TestVerifyHostname still
provides a smoke test checking that we can in fact talk TLS.

Updates #32978

Change-Id: I63505e22ff7704f25ad700d46e4ff14850ba5d3c
Reviewed-on: https://go-review.googlesource.com/c/go/+/186239
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
(cherry-picked from 20e4540)
Reviewed-on: https://go-review.googlesource.com/c/go/+/186279
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
@golang golang locked and limited conversation to collaborators Jul 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge release-blocker Soon This needs to be done soon. (regressions, serious bugs, outages) Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
None yet
Milestone
Go1.13
Development

No branches or pull requests
4 participants
@FiloSottile @dmitshur @bcmills @gopherbot