From 61f65bad48ce8fb229f6fe327a5da488194c2a71 Mon Sep 17 00:00:00 2001
From: Abhinav Gupta <abg@uber.com>
Date: Thu, 18 Nov 2021 11:40:09 -0800
Subject: [PATCH] TestAppRunTimeout: Fix OnStop_timeout flakiness

The OnStop_timeout test was flaky
because of the very small StartTimeout.

We would sometimes hit a condition where:
App.Run calls App.Start with a timeout of 1 millisecond.
This takes just long enough that App.Start gives up.
As a result, App.Run does not call App.Stop,
which causes the test to fail
because we don't see the Stop event.

Resolve the flakiness with the use of a mock clock.
Now, time doesn't advance while the start hook is running,
so we can be certain that the really long OnStop hook is invoked
and has a chance to time out.
---
 app_test.go | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/app_test.go b/app_test.go
index eac59c8be3..8b5e9c64ae 100644
--- a/app_test.go
+++ b/app_test.go
@@ -753,20 +753,16 @@ func TestTimeoutOptions(t *testing.T) {
 func TestAppRunTimeout(t *testing.T) {
 	t.Parallel()
 
-	// This context is only valid as long as this test is running.
-	// It lets us have goroutines that block forever
-	// (as far as the test is concerned) without leaking them.
-	testCtx, cancel := context.WithCancel(context.Background())
-	t.Cleanup(cancel)
+	mockClock := clock.NewMock()
 
 	// Fails with an error immediately.
 	failure := func(context.Context) error {
 		return errors.New("great sadness")
 	}
 
-	// Blocks forever--or at least until this test finishes running.
-	blockForever := func(context.Context) error {
-		<-testCtx.Done()
+	// Takes much longer than the application timeout.
+	takeVeryLong := func(context.Context) error {
+		mockClock.Add(time.Hour)
 		return errors.New("user should not see this")
 	}
 
@@ -782,7 +778,7 @@ func TestAppRunTimeout(t *testing.T) {
 			// Timeout starting an application.
 			desc: "OnStart timeout",
 			hooks: []Hook{
-				{OnStart: blockForever},
+				{OnStart: takeVeryLong},
 			},
 			wantEventType: &fxevent.Started{},
 		},
@@ -792,7 +788,7 @@ func TestAppRunTimeout(t *testing.T) {
 			hooks: []Hook{
 				// The hooks are separate because
 				// OnStop will not be run if that hook failed.
-				{OnStop: blockForever},
+				{OnStop: takeVeryLong},
 				{OnStart: failure},
 			},
 			wantEventType: &fxevent.Started{},
@@ -801,7 +797,7 @@ func TestAppRunTimeout(t *testing.T) {
 			// Timeout during a stop.
 			desc: "OnStop timeout",
 			hooks: []Hook{
-				{OnStop: blockForever},
+				{OnStop: takeVeryLong},
 			},
 			wantEventType: &fxevent.Stopped{},
 		},
@@ -838,9 +834,10 @@ func TestAppRunTimeout(t *testing.T) {
 			}
 
 			app, spy := NewSpied(
-				StartTimeout(time.Millisecond),
-				StopTimeout(time.Millisecond),
+				StartTimeout(time.Second),
+				StopTimeout(time.Second),
 				WithExit(exit),
+				WithClock(mockClock),
 				Invoke(func(lc Lifecycle) {
 					for _, h := range tt.hooks {
 						lc.Append(h)