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

cmd/compile: trybot flake in TestNexting #22206

Closed
ianlancetaylor opened this issue Oct 10, 2017 · 8 comments
Closed

cmd/compile: trybot flake in TestNexting #22206

ianlancetaylor opened this issue Oct 10, 2017 · 8 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@ianlancetaylor
Copy link
Contributor

From https://storage.googleapis.com/go-build-log/f7554b56/linux-amd64_e54e3ae9.log :

DID NOT MATCH [Inferior 1 (process 14469) exited normally]
(gdb) DID NOT MATCH [LWP 14528 exited]
[Inferior 1 (process 14528) exited normally]
(gdb) --- FAIL: TestNexting (9.86s)
debug_test.go:160: step/next histories differ, diff=
--- testdata/hist-opt.gdb-nexts 2017-10-10 22:06:01.000000000 +0000
+++ /tmp/debug_test377474953/hist-opt-test.gdb-nexts 2017-10-10 23:30:16.642615273 +0000
@@ -1,6 +1,8 @@
src/cmd/compile/internal/ssa/testdata/hist.go
35: func main() {
35: func main() {
+35: func main() {
+35: func main() {
36: hist := make([]int, 10)
37: var reader io.Reader = strings.NewReader(cannedInput) //gdb-dbg=(hist/A,cannedInput/A)
13: "strings"
FAIL
FAIL cmd/compile/internal/ssa 10.015s

@ianlancetaylor ianlancetaylor added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 10, 2017
@ianlancetaylor ianlancetaylor added this to the Go1.10 milestone Oct 10, 2017
@ianlancetaylor
Copy link
Contributor Author

This test is designed to be a change detector (https://testing.googleblog.com/2015/01/testing-on-toilet-change-detector-tests.html). Any number of things could cause this test to break, including minor changes to the fmt or bytes or strconv packages. I do not think we should be running this test as part of our automated testsuite. There will be too many unrelated breakages.

@gopherbot
Copy link

Change https://golang.org/cl/69790 mentions this issue: cmd/compile: disable TestNexting in short mode

gopherbot pushed a commit that referenced this issue Oct 11, 2017
Updates #22206

Change-Id: If75feddc01f8f86f294929fa7081c70eb15e581d
Reviewed-on: https://go-review.googlesource.com/69790
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
@dr2chase
Copy link
Contributor

I'd like to figure out what causes that intermittent quadruple-stop on main(). The test is designed to be a "change detector" because our current behavior for debugging code is so inadequate. I can write down a debugging correctness test that would not flake, because it would continuously fail.

@ianlancetaylor
Copy link
Contributor Author

I agree that it's hard to write tests for debug info. It's especially hard to write end-to-end tests that check how an external tool interprets the debug info.

But for the Go testsuite we really can't have tests that can change due to completely unrelated code changes, or that are flaky. You can write a test like yours while still controlling for these changes by using a program that does not call into the standard library at all.

@gopherbot
Copy link

Change https://golang.org/cl/69930 mentions this issue: cmd/compile: attempt to deflake debug_test.go

@dr2chase
Copy link
Contributor

The standard library call isn't supposed to appear in correct output (it's TestNexting, not TestStepping), and in theory we're trying to fix this in 1.10 (as part of the get-inlining-debugging-right plan). I could disable inlining for test.short till we get this fixed, that would do the job.

@dr2chase
Copy link
Contributor

The observed flake is also not the library dependence -- gdb appears to be unprepared for a really fast typist and in certain cases this causes lines to repeat. I made the test by-default tolerant of repeats in the line stream, and by-default disable inlining (but not other optimization) for testing with gdb which will also break that dependence (that we will not see anyway once we fix inlining).

gopherbot pushed a commit that referenced this issue Oct 13, 2017
Excluded when -short because it still runs relatively long,
but deflaked.

Removed timeouts from normal path and ensured that they were
not needed and that reference files did not change.

Use "tbreak" instead of "break" with gdb to reduce chance
of multiple hits on main.main.  (Seems not enough, but a
move in the right direction).

By default, testing ignores repeated lines that occur when
nexting.  This appears to sometimes be timing-dependent and
is the observed source of flakiness in testing so far.
Note that these can also be signs of a bug in the generated
debugging output, but it is one of the less-confusing bugs
that can occur.

By default, testing with gdb uses compilation with
inlining disabled to prevent dependence on library code
(it's a bug that library code is seen while Nexting, but
the bug is current behavior).

Also by default exclude all source files outside /testdata
to prevent accidental dependence on library code.  Note that
this is currently only applicable to dlv because (for the
debugging information we produce) gdb does not indicate a
change in the source file for inlined code.

Added flags -i and -r to make gdb testing compile with
inlining and be sensitive to repeats in the next stream.
This is for developer-testing and so we can describe these
problems in bug reports.

Updates #22206.

Change-Id: I9a30ebbc65aa0153fe77b1858cf19743bdc985e4
Reviewed-on: https://go-review.googlesource.com/69930
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@dr2chase
Copy link
Contributor

dr2chase commented Nov 30, 2017

I think this can be closed, because of short-mode disabled and because of e45e490 (also mentioned above).

@golang golang locked and limited conversation to collaborators Nov 30, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

3 participants