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

net: host string comparison in TestLookupGmailNS is case-sensitive, causes flaky test #34446

Closed
acln0 opened this issue Sep 21, 2019 · 3 comments
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone

Comments

@acln0
Copy link
Contributor

acln0 commented Sep 21, 2019

What version of Go are you using (go version)?

$ go version
go version devel +ecc7dd5469 Sat Sep 21 16:31:44 2019 +0000 linux/amd64

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN="/home/acln/bin"
GOCACHE="/home/acln/.cache/go-build"
GOENV="/home/acln/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/acln/src/gopath"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org"
GOROOT="/home/acln/src/go/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/acln/src/go/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/acln/src/go/go/src/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build058559209=/tmp/go-build -gno-record-gcc-switches"

What did you do?

I ran $ go test -run TestLookup in the net package.

What did you expect to see?

I expected the test to pass consistently.

What did you see instead?

The test passes sometimes, but also fails like so:

: forge:net ; /home/acln/src/go/go/bin/go test -run TestLookup
--- FAIL: TestLookupGmailNS (0.01s)
    lookup_test.go:197: got &{ns1.google.COm.}; want a record containing google.com.
    lookup_test.go:197: got &{ns2.google.COm.}; want a record containing google.com.
    lookup_test.go:197: got &{ns3.google.COm.}; want a record containing google.com.
    lookup_test.go:197: got &{ns4.google.COm.}; want a record containing google.com.

or even

: forge:net ; /home/acln/src/go/go/bin/go test -run TestLookup
--- FAIL: TestLookupGmailNS (0.01s)
    lookup_test.go:197: got &{ns2.google.coM.}; want a record containing google.com.
    lookup_test.go:197: got &{ns3.google.coM.}; want a record containing google.com.
    lookup_test.go:197: got &{ns1.google.coM.}; want a record containing google.com.
    lookup_test.go:197: got &{ns4.google.coM.}; want a record containing google.com.
    lookup_test.go:197: got &{ns1.google.CoM.}; want a record containing google.com.
    lookup_test.go:197: got &{ns2.google.CoM.}; want a record containing google.com.
    lookup_test.go:197: got &{ns3.google.CoM.}; want a record containing google.com.
    lookup_test.go:197: got &{ns4.google.CoM.}; want a record containing google.com.

This is the behavior I get when I use the default name server from my ISP. With Google (8.8.8.8) or CloudFlare (1.1.1.1) DNS, the test seems to pass without flaking. I am not expert on DNS, but it is my understanding that host names are case-insensitive. Perhaps this test should consider this fact.

If it seems sensible to use case-insensitive string comparison here, I can send a CL.

Interestingly, I couldn't get any other lookup test to fail when using the ISP's name server (which seems to consistently alter the case of the TLD for NS records it returns). I don't know if that is because that name server has a particular quirk, or because NS records are special in some way, or for some other reason that I cannot fathom.

@bcmills
Copy link
Contributor

bcmills commented Sep 23, 2019

CC @mikioh @bradfitz

@bcmills bcmills added ExpertNeeded NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Testing An issue that has been verified to require only test changes, not just a test failure. labels Sep 23, 2019
@bcmills bcmills added this to the Go1.14 milestone Sep 23, 2019
@bradfitz
Copy link
Contributor

Please send a CL.

@bcmills bcmills added help wanted NeedsFix The path to resolution is known, but the work has not been done. and removed ExpertNeeded labels Sep 23, 2019
@gopherbot gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 23, 2019
@gopherbot
Copy link

Change https://golang.org/cl/196801 mentions this issue: net: use case-insensitive host string comparison in TestLookupGmailNS

@golang golang locked and limited conversation to collaborators Sep 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
None yet
Development

No branches or pull requests

4 participants