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

net: re-enable or delete TestLookupLongTXT #22857

Open
0intro opened this issue Nov 23, 2017 · 17 comments
Open

net: re-enable or delete TestLookupLongTXT #22857

0intro opened this issue Nov 23, 2017 · 17 comments
Labels
help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unplanned

Comments

@0intro
Copy link
Member

0intro commented Nov 23, 2017

CL 79555 added TestLookupLongTXT. This test is failing on Plan 9.

--- FAIL: TestLookupLongTXT (0.05s)
	lookup_test.go:323: LookupTXT golang.rsc.io incorrect
		have ["abcdefghijklmnopqrstuvwxyABCDEFGHJIKLMNOPQRSTUVWXYabcdefghijklmnopqrstuvwxyABCDEFGHJIKLMNOPQRSTUVWXYabcdefghijklmnopqrstuvwxyABCDEFGHJIKLMNOPQRSTUVWXYabcdefghijklmnopqrstuvwxyABCDEFGHJIKLMNOPQRSTUVWXYabcdefghijklmnopqrstuvwxyABCDEFGHJIKLMNOPQRSTUVWXYabcdefghijklmnopqrstuvwxyABCDEFGHJIKLMNOPQRSTUVWXYabcdefghijklmnopqrstuvwxyABCDEFGHJIKLMNOPQRSTUVWXYabcdefghijklmnopqrstuvwxyABCDEFGHJIKLMNOPQRSTUVWXYabcdefghijklmnopqrstuvwxyABCDEFGHJIKLMNOPQRSTUVWXYabcdefghijklmnopqrstuvwxyABCDEFGHJIKLMNOPQRSTUVWXY"]
		want ["abcdefghijklmnopqrstuvwxyABCDEFGHJIKLMNOPQRSTUVWXYabcdefghijklmnopqrstuvwxyABCDEFGHJIKLMNOPQRSTUVWXYabcdefghijklmnopqrstuvwxyABCDEFGHJIKLMNOPQRSTUVWXYabcdefghijklmnopqrstuvwxyABCDEFGHJIKLMNOPQRSTUVWXYabcdefghijklmnopqrstuvwxyABCDEFGHJIKLMNOPQRSTUVWXYabcdefghijklmnopqrstuvwxyABCDEFGHJIKLMNOPQRSTUVWXYabcdefghijklmnopqrstuvwxyABCDEFGHJIKLMNOPQRSTUVWXYabcdefghijklmnopqrstuvwxyABCDEFGHJIKLMNOPQRSTUVWXYabcdefghijklmnopqrstuvwxyABCDEFGHJIKLMNOPQRSTUVWXYabcdefghijklmnopqrstuvwxyABCDEFGHJIKLMNOPQRSTUVWXY" "gophers rule"]
FAIL
FAIL	net	23.605s

See https://build.golang.org/log/d83b9b31e94b0b5093c0d2813f78105531e8de3e
@0intro 0intro added this to the Go1.10 milestone Nov 23, 2017
@0intro 0intro self-assigned this Nov 23, 2017
@0intro
Copy link
Member Author

0intro commented Nov 23, 2017

It seems the Plan 9 DNS resolver (ndb/dns) only returns a single TXT record. It probably should be fixed.

cpu% ndb/dnsquery
> rsc.io ip
rsc.io ip	216.239.38.21
rsc.io ip	216.239.34.21
rsc.io ip	216.239.32.21
rsc.io ip	216.239.36.21
> golang.rsc.io txt
golang.rsc.io txt	gophers rule
> gandi.net txt
gandi.net txt	google-site-verification: OyQ-vUKOBlZ2onEvCpyG7UGmQC1_tahPOUSNqCWYruo

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/79695 mentions this issue: net: skip TestLookupLongTXT on Plan 9

gopherbot pushed a commit that referenced this issue Nov 23, 2017
@0intro
net: skip TestLookupLongTXT on Plan 9
b72678f 
CL 79555 added TestLookupLongTXT. However, this test is
failing on Plan 9, because the DNS resolver (ndb/dns)
only returns a single TXT record.

Updates #22857.

Change-Id: I33cdc63a3d3de4d1c7f2684934316c44992fb9e2
Reviewed-on: https://go-review.googlesource.com/79695
Run-TryBot: David du Colombier <0intro@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@bradfitz bradfitz modified the milestones: Go1.10, Unplanned Nov 28, 2017
@bradfitz
Copy link
Contributor

@0intro, feel free to send plan9 fixes whenever, but I'm removing this from Go1.10. If the fix only touches plan9 stuff, I'm comfortable taking it even late in the cycle.

@mvdan
Copy link
Member

mvdan commented Jan 16, 2018

Different, potentially related failure: https://storage.googleapis.com/go-build-log/d38832a2/linux-amd64_b2d10b95.log

--- FAIL: TestLookupLongTXT (1.14s)
	lookup_test.go:342: LookupTXT golang.rsc.io incorrect
		have ["gophers rule"]
		want ["abcdefghijklmnopqrstuvwxyABCDEFGHJIKLMNOPQRSTUVWXYabcdefghijklmnopqrstuvwxyABCDEFGHJIKLMNOPQRSTUVWXYabcdefghijklmnopqrstuvwxyABCDEFGHJIKLMNOPQRSTUVWXYabcdefghijklmnopqrstuvwxyABCDEFGHJIKLMNOPQRSTUVWXYabcdefghijklmnopqrstuvwxyABCDEFGHJIKLMNOPQRSTUVWXYabcdefghijklmnopqrstuvwxyABCDEFGHJIKLMNOPQRSTUVWXYabcdefghijklmnopqrstuvwxyABCDEFGHJIKLMNOPQRSTUVWXYabcdefghijklmnopqrstuvwxyABCDEFGHJIKLMNOPQRSTUVWXYabcdefghijklmnopqrstuvwxyABCDEFGHJIKLMNOPQRSTUVWXYabcdefghijklmnopqrstuvwxyABCDEFGHJIKLMNOPQRSTUVWXY" "gophers rule"]

This happened on an unrelated CL, https://go-review.googlesource.com/c/go/+/77253 - linux-amd64 at d38832a2029f83e62a9ddfccfff6c2fc4e3a393e.

@bradfitz bradfitz changed the title net: TestLookupLongTXT failing on Plan 9 net: TestLookupLongTXT failing Feb 27, 2018
@bradfitz bradfitz added NeedsFix The path to resolution is known, but the work has not been done. and removed OS-Plan9 labels Feb 27, 2018
@bradfitz bradfitz modified the milestones: Unplanned, Go1.11 Feb 27, 2018
@bradfitz
Copy link
Contributor

This isn't just a plan9 issue. It's failing all over the place.

@ianlancetaylor and @rsc were discussing the problem recently. I think they understand what's happening here. Ian, can you summarize?

/cc @mikioh

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/97416 mentions this issue: net: skip flaky TestLookupLongTXT for now

@ianlancetaylor
Copy link
Contributor

I don't actually understand what is going on here. Maybe @rsc does. It seems to have something to do with how caches handle multiple TXT records.

gopherbot pushed a commit that referenced this issue Mar 2, 2018
@bradfitz
net: skip flaky TestLookupLongTXT for now
dcedcaa 
Flaky tests failing trybots help nobody.

Updates #22857

Change-Id: I87bc018651ab4fe02560a6d24c08a1d7ccd8ba37
Reviewed-on: https://go-review.googlesource.com/97416
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@ianlancetaylor ianlancetaylor modified the milestones: Go1.11, Go1.12 Jun 27, 2018
@ianlancetaylor
Copy link
Contributor

This code was rewritten as part of #21160. We should try enabling the test again at the start of the 1.13 cycle.

@ianlancetaylor ianlancetaylor added the early-in-cycle A change that should be done early in the 3 month dev cycle. label Dec 18, 2018
@ianlancetaylor ianlancetaylor modified the milestones: Go1.12, Go1.13 Dec 18, 2018
@ianlancetaylor ianlancetaylor mentioned this issue Dec 18, 2018
Open
@ianlancetaylor
Copy link
Contributor

See also #23873.

@0intro 0intro mentioned this issue Jan 13, 2019
Open
@gopherbot
Copy link
Contributor

This issue is currently labeled as early-in-cycle for Go 1.19.
That time is now, so a friendly reminder to look at it again.

@bcmills bcmills changed the title net: TestLookupLongTXT failing net: re-enable or delete TestLookupLongTXT May 13, 2022
@bcmills bcmills added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label May 13, 2022
@gopherbot gopherbot removed the NeedsFix The path to resolution is known, but the work has not been done. label May 13, 2022
@dmitshur dmitshur removed the early-in-cycle A change that should be done early in the 3 month dev cycle. label May 30, 2022
@dmitshur
Copy link
Contributor

dmitshur commented Jun 1, 2022

@neild This is another net test that needs a decision for 1.19 (currently blocking beta1), if you can take a look please. Thanks.

@neild neild modified the milestones: Go1.19, Go1.20 Jun 1, 2022
@neild neild added the early-in-cycle A change that should be done early in the 3 month dev cycle. label Jun 1, 2022
@neild
Copy link
Contributor

neild commented Jun 1, 2022

To summarize: TestLookupLongTXT was disabled a long time ago because it was flaky. The underlying code has since been rewritten. We should reenable the test to see if it's fixed now.

This should have been enabled early in the 1.19 cycle. It's now late in the 1.19 cycle. Kicking back to 1.20.

@neild neild self-assigned this Jun 1, 2022
@gopherbot
Copy link
Contributor

This issue is currently labeled as early-in-cycle for Go 1.20.
That time is now, so a friendly reminder to look at it again.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/423454 mentions this issue: net: reenable TestLookupLongTXT

@gopherbot gopherbot moved this to Done in Release Blockers Aug 17, 2022
@neild neild reopened this Aug 17, 2022
@neild
Copy link
Contributor

neild commented Aug 17, 2022

Looks like the same issue as described in #22857 (comment) is still occurring:
https://build.golang.org/log/c7feb48ee4f491ec0c89462361e4b4fc763cd0ba

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/424535 mentions this issue: net: revert "reenable TestLookupLongTXT"

gopherbot pushed a commit that referenced this issue Aug 17, 2022
@neild
net: revert "reenable TestLookupLongTXT"
ebda5a7 
Test is still flaky.

For #22857

Change-Id: Ic0d979778eb4c2d3779b18a983e7077789ae08a4
Reviewed-on: https://go-review.googlesource.com/c/go/+/424535
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
@neild neild removed early-in-cycle A change that should be done early in the 3 month dev cycle. release-blocker labels Aug 18, 2022
@neild neild removed this from Release Blockers Aug 18, 2022
@neild neild modified the milestones: Go1.20, Unplanned Aug 18, 2022
@neild neild removed their assignment Aug 18, 2022
@dmitshur dmitshur added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Aug 19, 2022
@heschi heschi mentioned this issue Aug 19, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
9 participants
@bradfitz @neild @rsc @dmitshur @ianlancetaylor @mvdan @bcmills @0intro @gopherbot