From ae652a4ac9354fef81610ca616b872262ea51281 Mon Sep 17 00:00:00 2001
From: Henrique Vicente <henriquevicente@gmail.com>
Date: Mon, 16 Nov 2020 03:09:31 +0100
Subject: [PATCH] os/signal: fix flaky tests for NotifyContext.

Test failures started to happen sporadically on some builds after the introduction of NotifyContext.
To make these tests more robust and avoid the risk of crosstalk we run them in a separate process.

Fixes #41561.

Change-Id: Ia7af105c316afd11765358f1e5e253ccfe2adc2b
Reviewed-on: https://go-review.googlesource.com/c/go/+/270198
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Bryan C. Mills <bcmills@google.com>
Trust: Cherry Zhang <cherryyz@google.com>
---
 src/os/signal/signal_test.go | 102 ++++++++++++++++++++---------------
 1 file changed, 60 insertions(+), 42 deletions(-)

diff --git a/src/os/signal/signal_test.go b/src/os/signal/signal_test.go
index 8945cbfccb..bbc68af9fb 100644
--- a/src/os/signal/signal_test.go
+++ b/src/os/signal/signal_test.go
@@ -675,22 +675,68 @@ func TestTime(t *testing.T) {
 	<-done
 }
 
-func TestNotifyContext(t *testing.T) {
-	c, stop := NotifyContext(context.Background(), syscall.SIGINT)
-	defer stop()
-
-	if want, got := "signal.NotifyContext(context.Background, [interrupt])", fmt.Sprint(c); want != got {
-		t.Errorf("c.String() = %q, want %q", got, want)
-	}
+var (
+	checkNotifyContext = flag.Bool("check_notify_ctx", false, "if true, TestNotifyContext will fail if SIGINT is not received.")
+	ctxNotifyTimes     = flag.Int("ctx_notify_times", 1, "number of times a SIGINT signal should be received")
+)
 
-	syscall.Kill(syscall.Getpid(), syscall.SIGINT)
-	select {
-	case <-c.Done():
-		if got := c.Err(); got != context.Canceled {
-			t.Errorf("c.Err() = %q, want %q", got, context.Canceled)
+func TestNotifyContextNotifications(t *testing.T) {
+	if *checkNotifyContext {
+		ctx, _ := NotifyContext(context.Background(), syscall.SIGINT)
+		// We want to make sure not to be calling Stop() internally on NotifyContext() when processing a received signal.
+		// Being able to wait for a number of received system signals allows us to do so.
+		var wg sync.WaitGroup
+		n := *ctxNotifyTimes
+		wg.Add(n)
+		for i := 0; i < n; i++ {
+			go func() {
+				syscall.Kill(syscall.Getpid(), syscall.SIGINT)
+				wg.Done()
+			}()
 		}
-	case <-time.After(time.Second):
-		t.Errorf("timed out waiting for context to be done after SIGINT")
+		wg.Wait()
+		<-ctx.Done()
+		fmt.Print("received SIGINT")
+		// Sleep to give time to simultaneous signals to reach the process.
+		// These signals must be ignored given stop() is not called on this code.
+		// We want to guarantee a SIGINT doesn't cause a premature termination of the program.
+		time.Sleep(settleTime)
+		return
+	}
+
+	t.Parallel()
+	testCases := []struct {
+		name string
+		n    int // number of times a SIGINT should be notified.
+	}{
+		{"once", 1},
+		{"multiple", 10},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			var subTimeout time.Duration
+			if deadline, ok := t.Deadline(); ok {
+				subTimeout := time.Until(deadline)
+				subTimeout -= subTimeout / 10 // Leave 10% headroom for cleaning up subprocess.
+			}
+
+			args := []string{
+				"-test.v",
+				"-test.run=TestNotifyContextNotifications$",
+				"-check_notify_ctx",
+				fmt.Sprintf("-ctx_notify_times=%d", tc.n),
+			}
+			if subTimeout != 0 {
+				args = append(args, fmt.Sprintf("-test.timeout=%v", subTimeout))
+			}
+			out, err := exec.Command(os.Args[0], args...).CombinedOutput()
+			if err != nil {
+				t.Errorf("ran test with -check_notify_ctx_notification and it failed with %v.\nOutput:\n%s", err, out)
+			}
+			if want := []byte("received SIGINT"); !bytes.Contains(out, want) {
+				t.Errorf("got %q, wanted %q", out, want)
+			}
+		})
 	}
 }
 
@@ -768,34 +814,6 @@ func TestNotifyContextPrematureCancelParent(t *testing.T) {
 	}
 }
 
-func TestNotifyContextSimultaneousNotifications(t *testing.T) {
-	c, stop := NotifyContext(context.Background(), syscall.SIGINT)
-	defer stop()
-
-	if want, got := "signal.NotifyContext(context.Background, [interrupt])", fmt.Sprint(c); want != got {
-		t.Errorf("c.String() = %q, want %q", got, want)
-	}
-
-	var wg sync.WaitGroup
-	n := 10
-	wg.Add(n)
-	for i := 0; i < n; i++ {
-		go func() {
-			syscall.Kill(syscall.Getpid(), syscall.SIGINT)
-			wg.Done()
-		}()
-	}
-	wg.Wait()
-	select {
-	case <-c.Done():
-		if got := c.Err(); got != context.Canceled {
-			t.Errorf("c.Err() = %q, want %q", got, context.Canceled)
-		}
-	case <-time.After(time.Second):
-		t.Errorf("expected context to be canceled")
-	}
-}
-
 func TestNotifyContextSimultaneousStop(t *testing.T) {
 	c, stop := NotifyContext(context.Background(), syscall.SIGINT)
 	defer stop()
-- 
2.48.1