From 5461acf3614660747be8bd52c6f87f5b1055c45e Mon Sep 17 00:00:00 2001 From: Alessandro Arzilli Date: Tue, 18 Aug 2020 02:17:39 +0200 Subject: [PATCH] tests: relax tests that use goroutinestackprog (#2136) Commit 1ee8d5c reviewed in Pull Request #1960 relaxed some tests using goroutinestackprog but missed others. Fixes some test flakiness that isn't relevant. --- pkg/proc/proc_test.go | 16 +++++++++- pkg/terminal/command_test.go | 52 ++++++++++++++++++++++++++++--- service/test/integration1_test.go | 16 +++++++++- 3 files changed, 78 insertions(+), 6 deletions(-) diff --git a/pkg/proc/proc_test.go b/pkg/proc/proc_test.go index c69c52cb..a156334c 100644 --- a/pkg/proc/proc_test.go +++ b/pkg/proc/proc_test.go @@ -1293,9 +1293,23 @@ func TestFrameEvaluation(t *testing.T) { found[vval] = true } + firsterr := false + if goversion.VersionAfterOrEqual(runtime.Version(), 1, 14) { + // We try to make sure that all goroutines are stopped at a sensible place + // before reading their stacktrace, but due to the nature of the test + // program there is no guarantee that we always find them in a reasonable + // state. + // Asynchronous preemption in Go 1.14 exacerbates this problem, to avoid + // unnecessary flakiness allow a single goroutine to be in a bad state. + firsterr = true + } for i := range found { if !found[i] { - t.Fatalf("Goroutine %d not found\n", i) + if firsterr { + firsterr = false + } else { + t.Fatalf("Goroutine %d not found\n", i) + } } } diff --git a/pkg/terminal/command_test.go b/pkg/terminal/command_test.go index f9e7c59a..359b6f11 100644 --- a/pkg/terminal/command_test.go +++ b/pkg/terminal/command_test.go @@ -341,6 +341,20 @@ func TestScopePrefix(t *testing.T) { const goroutinesLinePrefix = " Goroutine " const goroutinesCurLinePrefix = "* Goroutine " test.AllowRecording(t) + + tgtAgoroutineCount := 10 + + if goversion.VersionAfterOrEqual(runtime.Version(), 1, 14) { + // We try to make sure that all goroutines are stopped at a sensible place + // before reading their stacktrace, but due to the nature of the test + // program there is no guarantee that we always find them in a reasonable + // state. + // Asynchronous preemption in Go 1.14 exacerbates this problem, to avoid + // unnecessary flakiness reduce the target count to 9, allowing one + // goroutine to be in a bad state. + tgtAgoroutineCount = 9 + } + withTestTerminal("goroutinestackprog", t, func(term *FakeTerminal) { term.MustExec("b stacktraceme") term.MustExec("continue") @@ -393,7 +407,7 @@ func TestScopePrefix(t *testing.T) { } } } - if len(agoroutines)+extraAgoroutines < 10 { + if len(agoroutines)+extraAgoroutines < tgtAgoroutineCount { t.Fatalf("Output of goroutines did not have 10 goroutines stopped on main.agoroutine (%d+%d found): %q", len(agoroutines), extraAgoroutines, goroutinesOut) } } @@ -437,8 +451,12 @@ func TestScopePrefix(t *testing.T) { seen[ival] = true } + firsterr := tgtAgoroutineCount != 10 + for i := range seen { - if !seen[i] { + if firsterr { + firsterr = false + } else if !seen[i] { t.Fatalf("goroutine %d not found", i) } } @@ -518,8 +536,21 @@ func TestOnPrefix(t *testing.T) { } } + firsterr := false + if goversion.VersionAfterOrEqual(runtime.Version(), 1, 14) { + // We try to make sure that all goroutines are stopped at a sensible place + // before reading their stacktrace, but due to the nature of the test + // program there is no guarantee that we always find them in a reasonable + // state. + // Asynchronous preemption in Go 1.14 exacerbates this problem, to avoid + // unnecessary flakiness allow a single goroutine to be in a bad state. + firsterr = true + } + for i := range seen { - if !seen[i] { + if firsterr { + firsterr = false + } else if !seen[i] { t.Fatalf("Goroutine %d not seen\n", i) } } @@ -577,8 +608,21 @@ func TestOnPrefixLocals(t *testing.T) { } } + firsterr := false + if goversion.VersionAfterOrEqual(runtime.Version(), 1, 14) { + // We try to make sure that all goroutines are stopped at a sensible place + // before reading their stacktrace, but due to the nature of the test + // program there is no guarantee that we always find them in a reasonable + // state. + // Asynchronous preemption in Go 1.14 exacerbates this problem, to avoid + // unnecessary flakiness allow a single goroutine to be in a bad state. + firsterr = true + } + for i := range seen { - if !seen[i] { + if firsterr { + firsterr = false + } else if !seen[i] { t.Fatalf("Goroutine %d not seen\n", i) } } diff --git a/service/test/integration1_test.go b/service/test/integration1_test.go index 08966c52..89b5f413 100644 --- a/service/test/integration1_test.go +++ b/service/test/integration1_test.go @@ -751,9 +751,23 @@ func Test1ClientServer_FullStacktrace(t *testing.T) { } } + firsterr := false + if goversion.VersionAfterOrEqual(runtime.Version(), 1, 14) { + // We try to make sure that all goroutines are stopped at a sensible place + // before reading their stacktrace, but due to the nature of the test + // program there is no guarantee that we always find them in a reasonable + // state. + // Asynchronous preemption in Go 1.14 exacerbates this problem, to avoid + // unnecessary flakiness allow a single goroutine to be in a bad state. + firsterr = true + } for i := range found { if !found[i] { - t.Fatalf("Goroutine %d not found", i) + if firsterr { + firsterr = false + } else { + t.Fatalf("Goroutine %d not found", i) + } } }