Refactor renewer tests (#4433)

api/renewer_integration_test.go

@@ -13,9 +13,6 @@ func TestRenewer_Renew(t *testing.T) {
 	client, vaultDone := testVaultServer(t)
 	defer vaultDone()
 
-	pgURL, pgDone := testPostgresDB(t)
-	defer pgDone()
-
 	t.Run("group", func(t *testing.T) {
 		t.Run("kv", func(t *testing.T) {
 			t.Parallel()
@@ -92,6 +89,9 @@ func TestRenewer_Renew(t *testing.T) {
 		t.Run("database", func(t *testing.T) {
 			t.Parallel()
 
+			pgURL, pgDone := testPostgresDB(t)
+			defer pgDone()
+
 			if err := client.Sys().Mount("database", &api.MountInput{
 				Type: "database",
 			}); err != nil {
@@ -129,9 +129,21 @@ func TestRenewer_Renew(t *testing.T) {
 			go v.Renew()
 			defer v.Stop()
 
+			done, renewed := false, false
+			for {
+				if done {
+					break
+				}
 				select {
 				case err := <-v.DoneCh():
+					if renewed {
+						// If we renewed but there's an error, we fail
+						if err != nil {
+							t.Fatalf("renewal failed with an error: %v", err)
+						}
+					} else {
 						t.Errorf("should have renewed once before returning: %s", err)
+					}
 				case renew := <-v.RenewCh():
 					if renew == nil {
 						t.Fatal("renew is nil")
@@ -142,24 +154,12 @@ func TestRenewer_Renew(t *testing.T) {
 					if renew.Secret.LeaseDuration > 5 {
 						t.Errorf("expected lease to <= 5s: %#v", renew)
 					}
+					renewed = true
 				case <-time.After(5 * time.Second):
+					if !renewed {
 						t.Errorf("no renewal")
 					}
-
+					done = true
-		outer:
-			for {
-				select {
-				case err := <-v.DoneCh():
-					if err != nil {
-						t.Fatal(err)
-					}
-					break outer
-				case renew := <-v.RenewCh():
-					t.Logf("renew called, remaining lease duration: %d", renew.Secret.LeaseDuration)
-					continue outer
-				case <-time.After(5 * time.Second):
-					t.Errorf("no data")
-					break outer
 				}
 			}
 		})
@@ -169,7 +169,7 @@ func TestRenewer_Renew(t *testing.T) {
 
 			secret, err := client.Auth().Token().Create(&api.TokenCreateRequest{
 				Policies:       []string{"default"},
-				TTL:            "3s",
+				TTL:            "5s",
 				ExplicitMaxTTL: "10s",
 			})
 			if err != nil {
@@ -185,9 +185,22 @@ func TestRenewer_Renew(t *testing.T) {
 			go v.Renew()
 			defer v.Stop()
 
+			renewed, done := false, false
+			for {
+				if done {
+					break
+				}
 				select {
 				case err := <-v.DoneCh():
+					if renewed {
+						// If we renewed but there's an error, we fail
+						if err != nil {
+							t.Fatalf("renewal failed with an error: %v", err)
+						}
+					} else {
 						t.Errorf("should have renewed once before returning: %s", err)
+					}
+					done = true
 				case renew := <-v.RenewCh():
 					if renew == nil {
 						t.Fatal("renew is nil")
@@ -198,8 +211,8 @@ func TestRenewer_Renew(t *testing.T) {
 					if !renew.Secret.Auth.Renewable {
 						t.Errorf("expected lease to be renewable: %#v", renew)
 					}
-				if renew.Secret.Auth.LeaseDuration > 3 {
+					if renew.Secret.Auth.LeaseDuration > 5 {
-					t.Errorf("expected lease to < 3s: %#v", renew)
+						t.Errorf("expected lease to <= 5s: %#v", renew)
 					}
 					if renew.Secret.Auth.ClientToken == "" {
 						t.Error("expected a client token")
@@ -207,23 +220,12 @@ func TestRenewer_Renew(t *testing.T) {
 					if renew.Secret.Auth.Accessor == "" {
 						t.Error("expected an accessor")
 					}
-			case <-time.After(5 * time.Second):
+					renewed = true
+				case <-time.After(10 * time.Second):
+					if !renewed {
 						t.Errorf("no renewal")
 					}
-
+					done = true
-		outer:
-			for {
-				select {
-				case err := <-v.DoneCh():
-					if err != nil {
-						t.Fatal(err)
-					}
-					break outer
-				case <-v.RenewCh():
-					continue outer
-				case <-time.After(5 * time.Second):
-					t.Errorf("no data")
-					break outer
 				}
 			}
 		})