-
Notifications
You must be signed in to change notification settings - Fork 17.7k
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
runtime: move tests for runtime-gdb.py to a separate package and do not run them during all.bash #39204
Comments
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. |
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. |
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 ¹ go/src/runtime/runtime-gdb_test.go Lines 22 to 50 in 5c802c1
|
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. |
FYI, maybe related to |
I just encountered one on tip:
|
Moving to Backlog. |
@golang/runtime, can you please take another look at this in triage? I keep hitting #58698 in TryBots. I expect that external Go users may hit these test flakes in their own projects when running |
Change https://go.dev/cl/499975 mentions this issue: |
One issue simply has a reworded message, probably from a new version of GDB. Another is a new issue. Fixes #60553. Fixes #58698. Updates #39204. Change-Id: I8389aa981fab5421f57ee761bfb5e1dd237709ed Reviewed-on: https://go-review.googlesource.com/c/go/+/499975 Reviewed-by: Bryan Mills <bcmills@google.com> Run-TryBot: Michael Pratt <mpratt@google.com> Auto-Submit: Michael Pratt <mpratt@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Austin Clements <austin@google.com>
The tests in
runtime-gdb_test.go
, for the GDB bindings inruntime-gdb.py
, have historically been very flaky. They are currently skipped on a large fraction of platforms due to: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:
all.bash
#61487#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 justSkip
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 theruntime
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 thegdb
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 oneSkip
based on a check for agdb
executable at a sufficiently recent version.The text was updated successfully, but these errors were encountered: