states/remote: use t.Run in table-based tests

These tests were originally written long before Go supported subtests
explicitly, but now that we have t.Run we can avoid the prior problem
that one test failing would mask all of the others that followed it.

Now we'll always run all of them, potentially collecting more errors in
a single run so we can have more context to debug with and potentially
fix them all in a single step rather than one by one.
This commit is contained in:
Martin Atkins 2022-06-17 12:00:42 -07:00
parent ad5ac89461
commit 69aa0a2b1f

View File

@ -245,28 +245,30 @@ func TestStatePersist(t *testing.T) {
// Run tests in order.
for _, tc := range testCases {
s, cleanup := tc.mutationFunc(mgr)
t.Run(tc.name, func(t *testing.T) {
s, cleanup := tc.mutationFunc(mgr)
if err := mgr.WriteState(s); err != nil {
t.Fatalf("failed to WriteState for %q: %s", tc.name, err)
}
if err := mgr.PersistState(); err != nil {
t.Fatalf("failed to PersistState for %q: %s", tc.name, err)
}
if err := mgr.WriteState(s); err != nil {
t.Fatalf("failed to WriteState for %q: %s", tc.name, err)
}
if err := mgr.PersistState(); err != nil {
t.Fatalf("failed to PersistState for %q: %s", tc.name, err)
}
if tc.isRequested(t) {
// Get captured request from the mock client log
// based on the index of the current test
if logIdx >= len(mockClient.log) {
t.Fatalf("request lock and index are out of sync on %q: idx=%d len=%d", tc.name, logIdx, len(mockClient.log))
if tc.isRequested(t) {
// Get captured request from the mock client log
// based on the index of the current test
if logIdx >= len(mockClient.log) {
t.Fatalf("request lock and index are out of sync on %q: idx=%d len=%d", tc.name, logIdx, len(mockClient.log))
}
loggedRequest := mockClient.log[logIdx]
logIdx++
if diff := cmp.Diff(tc.expectedRequest, loggedRequest); len(diff) > 0 {
t.Fatalf("incorrect client requests for %q:\n%s", tc.name, diff)
}
}
loggedRequest := mockClient.log[logIdx]
logIdx++
if diff := cmp.Diff(tc.expectedRequest, loggedRequest); len(diff) > 0 {
t.Fatalf("incorrect client requests for %q:\n%s", tc.name, diff)
}
}
cleanup()
cleanup()
})
}
logCnt := len(mockClient.log)
if logIdx != logCnt {
@ -395,37 +397,39 @@ func TestWriteStateForMigration(t *testing.T) {
logIdx := 0
for _, tc := range testCases {
sf := tc.stateFile(mgr)
err := mgr.WriteStateForMigration(sf, tc.force)
shouldError := tc.expectedError != ""
t.Run(tc.name, func(t *testing.T) {
sf := tc.stateFile(mgr)
err := mgr.WriteStateForMigration(sf, tc.force)
shouldError := tc.expectedError != ""
// If we are expecting and error check it and move on
if shouldError {
if err == nil {
t.Fatalf("test case %q should have failed with error %q", tc.name, tc.expectedError)
} else if err.Error() != tc.expectedError {
t.Fatalf("test case %q expected error %q but got %q", tc.name, tc.expectedError, err)
// If we are expecting and error check it and move on
if shouldError {
if err == nil {
t.Fatalf("test case %q should have failed with error %q", tc.name, tc.expectedError)
} else if err.Error() != tc.expectedError {
t.Fatalf("test case %q expected error %q but got %q", tc.name, tc.expectedError, err)
}
return
}
continue
}
if err != nil {
t.Fatalf("test case %q failed: %v", tc.name, err)
}
if err != nil {
t.Fatalf("test case %q failed: %v", tc.name, err)
}
// At this point we should just do a normal write and persist
// as would happen from the CLI
mgr.WriteState(mgr.State())
mgr.PersistState()
// At this point we should just do a normal write and persist
// as would happen from the CLI
mgr.WriteState(mgr.State())
mgr.PersistState()
if logIdx >= len(mockClient.log) {
t.Fatalf("request lock and index are out of sync on %q: idx=%d len=%d", tc.name, logIdx, len(mockClient.log))
}
loggedRequest := mockClient.log[logIdx]
logIdx++
if diff := cmp.Diff(tc.expectedRequest, loggedRequest); len(diff) > 0 {
t.Fatalf("incorrect client requests for %q:\n%s", tc.name, diff)
}
if logIdx >= len(mockClient.log) {
t.Fatalf("request lock and index are out of sync on %q: idx=%d len=%d", tc.name, logIdx, len(mockClient.log))
}
loggedRequest := mockClient.log[logIdx]
logIdx++
if diff := cmp.Diff(tc.expectedRequest, loggedRequest); len(diff) > 0 {
t.Fatalf("incorrect client requests for %q:\n%s", tc.name, diff)
}
})
}
logCnt := len(mockClient.log)
@ -551,43 +555,45 @@ func TestWriteStateForMigrationWithForcePushClient(t *testing.T) {
logIdx := 0
for _, tc := range testCases {
// Always reset client to not be force pushing
mockClient.force = false
sf := tc.stateFile(mgr)
err := mgr.WriteStateForMigration(sf, tc.force)
shouldError := tc.expectedError != ""
t.Run(tc.name, func(t *testing.T) {
// Always reset client to not be force pushing
mockClient.force = false
sf := tc.stateFile(mgr)
err := mgr.WriteStateForMigration(sf, tc.force)
shouldError := tc.expectedError != ""
// If we are expecting and error check it and move on
if shouldError {
if err == nil {
t.Fatalf("test case %q should have failed with error %q", tc.name, tc.expectedError)
} else if err.Error() != tc.expectedError {
t.Fatalf("test case %q expected error %q but got %q", tc.name, tc.expectedError, err)
// If we are expecting and error check it and move on
if shouldError {
if err == nil {
t.Fatalf("test case %q should have failed with error %q", tc.name, tc.expectedError)
} else if err.Error() != tc.expectedError {
t.Fatalf("test case %q expected error %q but got %q", tc.name, tc.expectedError, err)
}
return
}
continue
}
if err != nil {
t.Fatalf("test case %q failed: %v", tc.name, err)
}
if err != nil {
t.Fatalf("test case %q failed: %v", tc.name, err)
}
if tc.force && !mockClient.force {
t.Fatalf("test case %q should have enabled force push", tc.name)
}
if tc.force && !mockClient.force {
t.Fatalf("test case %q should have enabled force push", tc.name)
}
// At this point we should just do a normal write and persist
// as would happen from the CLI
mgr.WriteState(mgr.State())
mgr.PersistState()
// At this point we should just do a normal write and persist
// as would happen from the CLI
mgr.WriteState(mgr.State())
mgr.PersistState()
if logIdx >= len(mockClient.log) {
t.Fatalf("request lock and index are out of sync on %q: idx=%d len=%d", tc.name, logIdx, len(mockClient.log))
}
loggedRequest := mockClient.log[logIdx]
logIdx++
if diff := cmp.Diff(tc.expectedRequest, loggedRequest); len(diff) > 0 {
t.Fatalf("incorrect client requests for %q:\n%s", tc.name, diff)
}
if logIdx >= len(mockClient.log) {
t.Fatalf("request lock and index are out of sync on %q: idx=%d len=%d", tc.name, logIdx, len(mockClient.log))
}
loggedRequest := mockClient.log[logIdx]
logIdx++
if diff := cmp.Diff(tc.expectedRequest, loggedRequest); len(diff) > 0 {
t.Fatalf("incorrect client requests for %q:\n%s", tc.name, diff)
}
})
}
logCnt := len(mockClient.log)