TestLifetimeWatcher: Address race condition in test assertions (#15969)

- If the timing is correct, a delay in the test's select might see the
   doneCh signal before the renew channels signal. If that happens, the
   test fails as it assumes we will receive signals across different
   channels in order.
 - Rework the test to make sure that we read from the renew channel if expected
   and the done channel so that any errors might not be escaping from detection
   on a renew.

api/renewer_test.go

@@ -192,27 +192,44 @@ func TestLifetimeWatcher(t *testing.T) {
 			}()
 			defer v.Stop()
 
-			select {
+			receivedRenewal := false
-			case <-time.After(tc.maxTestTime):
+			receivedDone := false
-				t.Fatalf("renewal didn't happen")
+		ChannelLoop:
-			case r := <-v.RenewCh():
+			for {
-				if !tc.expectRenewal {
+				select {
-					t.Fatal("expected no renewals")
+				case <-time.After(tc.maxTestTime):
-				}
+					t.Fatalf("renewal didn't happen")
-				if r.Secret != renewedSecret {
+				case r := <-v.RenewCh():
-					t.Fatalf("expected secret %v, got %v", renewedSecret, r.Secret)
+					if !tc.expectRenewal {
-				}
+						t.Fatal("expected no renewals")
-			case err := <-doneCh:
+					}
-				if tc.expectError != nil && !errors.Is(err, tc.expectError) {
+					if r.Secret != renewedSecret {
-					t.Fatalf("expected error %q, got: %v", tc.expectError, err)
+						t.Fatalf("expected secret %v, got %v", renewedSecret, r.Secret)
-				}
+					}
-				if tc.expectError == nil && err != nil {
+					receivedRenewal = true
-					t.Fatalf("expected no error, got: %v", err)
+					if !receivedDone {
-				}
+						continue ChannelLoop
-				if tc.expectRenewal {
+					}
-					t.Fatalf("expected at least one renewal, got donech result: %v", err)
+					break ChannelLoop
+				case err := <-doneCh:
+					receivedDone = true
+					if tc.expectError != nil && !errors.Is(err, tc.expectError) {
+						t.Fatalf("expected error %q, got: %v", tc.expectError, err)
+					}
+					if tc.expectError == nil && err != nil {
+						t.Fatalf("expected no error, got: %v", err)
+					}
+					if tc.expectRenewal && !receivedRenewal {
+						// We might have received the stop before the renew call on the channel.
+						continue ChannelLoop
+					}
+					break ChannelLoop
 				}
 			}
+
+			if tc.expectRenewal && !receivedRenewal {
+				t.Fatalf("expected at least one renewal, got none.")
+			}
 		})
 	}
 }