Go: runtime: move tests for runtime-gdb.py to misc/ and do not run them during all.bash

Created on 21 May 2020 · 5Comments · Source: golang/go

The tests in runtime-gdb_test.go, for the GDB bindings in runtime-gdb.py, have historically been very flaky. They are currently skipped on a large fraction of platforms due to:

#15603
#17366
#18784
#20821
#21380 / #22021 / #22687
#22893
#25939
#28558
#29508
#35710
and what appears to be an untracked issue for 'darwin': a skip was added in CL 20193, but neither the code itself nor any of the commentary I could find on the code review explains why gdb does or did not work on that platform.

If we had an incoming change to add a test with this many skips today, I suspect that we would turn it down: it tests what is supposed to be cross-platform code, but only checks the actual behavior of that code for a narrow subset of users — _because_ that behavior does not work reliably across platforms.

But it doesn't work reliably on the remaining tested platforms either: despite all of those skips, the tests are still flaky today:

#24616
- Possibly fixed in CL 233942, which notes:

This part of the test has been flaky despite repeated attempts to fix it,
and it is unclear what exactly it is testing. Remove it.

#25697

#35743

#37366

#37405

#37794

#39021

#39025

#39228

#43068

39021 in particular describes a regression during the 1.15 cycle that is interfering with the SlowBots and TryBots on pending CLs, will likely interfere with release testing as well, and so far has resisted attempts at bisection because the test (and its failure mode) is highly nondeterministic.

If we consider the runtime “incorrect” if it does not work with gdb, then we should take the time to fix and maintain the Go bindings properly and portably — not just Skip their tests! — in the same way that we fix and maintain other features of the runtime across platforms. Otherwise, it is not appropriate to run the tests for those bindings as part of the tests for the runtime package, which is a dependency of most Go binaries (and is therefore likely to have its tests run in users' CI systems).

Most of the other Go project tests for integration with third-party tools are in the misc subdirectory. I propose that we move the gdb integration tests there.

I further propose that we should not run these tests as part of all.bash, run.bash, or on the Go project builders until such time as they can be made reliable, with only one Skip based on a check for a gdb executable at a sufficiently recent version.

Debugging NeedsDecision Testing

Source

bcmills

👍4 ❤1

Most helpful comment

I don't think this has to be a proposal, in that I don't think it has to go through the proposal committee. This does not affect any user visible API. I think the runtime package maintainers can decide.

ianlancetaylor on 21 May 2020

👍3

All 5 comments

ianlancetaylor on 21 May 2020

👍3

The tests in runtime-gdb_test.go, for the GDB bindings in runtime-gdb.py ...

TestGdbBacktrace doesn't actually load the python script. It just gets GDB to execute a Go program, sets a breakpoint and triggers a backtrace. It is one of the few places where we test that the debug information in a Go executable can be consumed by a debugger like GDB.

I'm not sure what the solution to the flakiness is but I think having basic GDB support is valuable and should be tested on platforms where GDB is available. In particular I think we should continue to run the tests on the builders wherever possible - even if it is just a subset of platforms.

mundaym on 25 May 2020

@mundaym

I'm not sure what the solution to the flakiness is but I think having basic GDB support is valuable and should be tested on platforms where GDB is available.

Note that we already _do not_ test basic GDB support on most platforms for which GDB is available, due to the bugs mentioned in the first section.¹ If we agree that that support is valuable, then we should remove those skips and fix the behavior on those platforms. If we don't fix the tests when they are broken, what's the point in running them?

Either way, I still don't think these tests belong with the runtime package: they are testing integration with a third-party tool (gdb), not the Go-observable behavior of the runtime package.

¹
https://github.com/golang/go/blob/5c802c13e88b700b9acaf390d495a92101214e2b/src/runtime/runtime-gdb_test.go#L22-L50

bcmills on 26 May 2020

This didn't happen for 1.15 (and the gdb tests seem to have gotten a bit more reliable, too). Changing milestone to 1.16.

ianlancetaylor on 26 Jun 2020