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

Comments

Projects
None yet
3 participants
@ianlancetaylor
Copy link
Contributor

commented Oct 10, 2017

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

This comment has been minimized.

Copy link
Contributor Author

commented Oct 11, 2017

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

This comment has been minimized.

Copy link

commented Oct 11, 2017

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

cmd/compile: disable TestNexting in short mode
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

This comment has been minimized.

Copy link
Contributor

commented Oct 11, 2017

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

This comment has been minimized.

Copy link
Contributor Author

commented Oct 11, 2017

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

This comment has been minimized.

Copy link

commented Oct 11, 2017

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

@dr2chase

This comment has been minimized.

Copy link
Contributor

commented Oct 11, 2017

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

This comment has been minimized.

Copy link
Contributor

commented Oct 11, 2017

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

cmd/compile: attempt to deflake debug_test.go
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

This comment has been minimized.

Copy link
Contributor

commented Nov 30, 2017

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

@dr2chase dr2chase closed this Nov 30, 2017

@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.
You can’t perform that action at this time.