Skip to content

Commit ae652a4

Browse files
henvicianlancetaylor
authored andcommitted
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 <[email protected]> TryBot-Result: Go Bot <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]> Trust: Bryan C. Mills <[email protected]> Trust: Cherry Zhang <[email protected]>
1 parent 740851b commit ae652a4

File tree

1 file changed

+60
-42
lines changed

1 file changed

+60
-42
lines changed

src/os/signal/signal_test.go

Lines changed: 60 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -675,22 +675,68 @@ func TestTime(t *testing.T) {
675675
<-done
676676
}
677677

678-
func TestNotifyContext(t *testing.T) {
679-
c, stop := NotifyContext(context.Background(), syscall.SIGINT)
680-
defer stop()
681-
682-
if want, got := "signal.NotifyContext(context.Background, [interrupt])", fmt.Sprint(c); want != got {
683-
t.Errorf("c.String() = %q, want %q", got, want)
684-
}
678+
var (
679+
checkNotifyContext = flag.Bool("check_notify_ctx", false, "if true, TestNotifyContext will fail if SIGINT is not received.")
680+
ctxNotifyTimes = flag.Int("ctx_notify_times", 1, "number of times a SIGINT signal should be received")
681+
)
685682

686-
syscall.Kill(syscall.Getpid(), syscall.SIGINT)
687-
select {
688-
case <-c.Done():
689-
if got := c.Err(); got != context.Canceled {
690-
t.Errorf("c.Err() = %q, want %q", got, context.Canceled)
683+
func TestNotifyContextNotifications(t *testing.T) {
684+
if *checkNotifyContext {
685+
ctx, _ := NotifyContext(context.Background(), syscall.SIGINT)
686+
// We want to make sure not to be calling Stop() internally on NotifyContext() when processing a received signal.
687+
// Being able to wait for a number of received system signals allows us to do so.
688+
var wg sync.WaitGroup
689+
n := *ctxNotifyTimes
690+
wg.Add(n)
691+
for i := 0; i < n; i++ {
692+
go func() {
693+
syscall.Kill(syscall.Getpid(), syscall.SIGINT)
694+
wg.Done()
695+
}()
691696
}
692-
case <-time.After(time.Second):
693-
t.Errorf("timed out waiting for context to be done after SIGINT")
697+
wg.Wait()
698+
<-ctx.Done()
699+
fmt.Print("received SIGINT")
700+
// Sleep to give time to simultaneous signals to reach the process.
701+
// These signals must be ignored given stop() is not called on this code.
702+
// We want to guarantee a SIGINT doesn't cause a premature termination of the program.
703+
time.Sleep(settleTime)
704+
return
705+
}
706+
707+
t.Parallel()
708+
testCases := []struct {
709+
name string
710+
n int // number of times a SIGINT should be notified.
711+
}{
712+
{"once", 1},
713+
{"multiple", 10},
714+
}
715+
for _, tc := range testCases {
716+
t.Run(tc.name, func(t *testing.T) {
717+
var subTimeout time.Duration
718+
if deadline, ok := t.Deadline(); ok {
719+
subTimeout := time.Until(deadline)
720+
subTimeout -= subTimeout / 10 // Leave 10% headroom for cleaning up subprocess.
721+
}
722+
723+
args := []string{
724+
"-test.v",
725+
"-test.run=TestNotifyContextNotifications$",
726+
"-check_notify_ctx",
727+
fmt.Sprintf("-ctx_notify_times=%d", tc.n),
728+
}
729+
if subTimeout != 0 {
730+
args = append(args, fmt.Sprintf("-test.timeout=%v", subTimeout))
731+
}
732+
out, err := exec.Command(os.Args[0], args...).CombinedOutput()
733+
if err != nil {
734+
t.Errorf("ran test with -check_notify_ctx_notification and it failed with %v.\nOutput:\n%s", err, out)
735+
}
736+
if want := []byte("received SIGINT"); !bytes.Contains(out, want) {
737+
t.Errorf("got %q, wanted %q", out, want)
738+
}
739+
})
694740
}
695741
}
696742

@@ -768,34 +814,6 @@ func TestNotifyContextPrematureCancelParent(t *testing.T) {
768814
}
769815
}
770816

771-
func TestNotifyContextSimultaneousNotifications(t *testing.T) {
772-
c, stop := NotifyContext(context.Background(), syscall.SIGINT)
773-
defer stop()
774-
775-
if want, got := "signal.NotifyContext(context.Background, [interrupt])", fmt.Sprint(c); want != got {
776-
t.Errorf("c.String() = %q, want %q", got, want)
777-
}
778-
779-
var wg sync.WaitGroup
780-
n := 10
781-
wg.Add(n)
782-
for i := 0; i < n; i++ {
783-
go func() {
784-
syscall.Kill(syscall.Getpid(), syscall.SIGINT)
785-
wg.Done()
786-
}()
787-
}
788-
wg.Wait()
789-
select {
790-
case <-c.Done():
791-
if got := c.Err(); got != context.Canceled {
792-
t.Errorf("c.Err() = %q, want %q", got, context.Canceled)
793-
}
794-
case <-time.After(time.Second):
795-
t.Errorf("expected context to be canceled")
796-
}
797-
}
798-
799817
func TestNotifyContextSimultaneousStop(t *testing.T) {
800818
c, stop := NotifyContext(context.Background(), syscall.SIGINT)
801819
defer stop()

0 commit comments

Comments
 (0)