diff --git a/client/client_test.go b/client/client_test.go index 693faaa..322d7f0 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -113,7 +113,7 @@ func TestStore(t *testing.T) { for _, v := range valid { if err := Store(mockProgramFn, &v); err != nil { - t.Fatal(err) + t.Error(err) } } @@ -123,7 +123,7 @@ func TestStore(t *testing.T) { for _, v := range invalid { if err := Store(mockProgramFn, &v); err == nil { - t.Fatalf("Expected error for server %s, got nil", v.ServerURL) + t.Errorf("Expected error for server %s, got nil", v.ServerURL) } } } @@ -152,10 +152,10 @@ func TestGet(t *testing.T) { } if c.Username != v.Username { - t.Fatalf("expected username `%s`, got %s", v.Username, c.Username) + t.Errorf("expected username `%s`, got %s", v.Username, c.Username) } if c.Secret != v.Secret { - t.Fatalf("expected secret `%s`, got %s", v.Secret, c.Secret) + t.Errorf("expected secret `%s`, got %s", v.Secret, c.Secret) } } @@ -165,10 +165,17 @@ func TestGet(t *testing.T) { serverURL string err string }{ - {missingCredsAddress, credentials.NewErrCredentialsNotFound().Error()}, - {invalidServerAddress, "error getting credentials - err: exited 1, out: `program failed`"}, - {"", fmt.Sprintf("error getting credentials - err: %s, out: `%s`", - missingServerURLErr.Error(), missingServerURLErr.Error())}, + { + serverURL: missingCredsAddress, + err: credentials.NewErrCredentialsNotFound().Error(), + }, + { + serverURL: invalidServerAddress, + err: "error getting credentials - err: exited 1, out: `program failed`", + }, + { + err: fmt.Sprintf("error getting credentials - err: %s, out: `%s`", missingServerURLErr.Error(), missingServerURLErr.Error()), + }, } for _, v := range invalid { @@ -177,7 +184,7 @@ func TestGet(t *testing.T) { t.Fatalf("Expected error for server %s, got nil", v.serverURL) } if err.Error() != v.err { - t.Fatalf("Expected error `%s`, got `%v`", v.err, err) + t.Errorf("Expected error `%s`, got `%v`", v.err, err) } } } @@ -192,11 +199,11 @@ func ExampleErase() { func TestErase(t *testing.T) { if err := Erase(mockProgramFn, validServerAddress); err != nil { - t.Fatal(err) + t.Error(err) } if err := Erase(mockProgramFn, invalidServerAddress); err == nil { - t.Fatalf("Expected error for server %s, got nil", invalidServerAddress) + t.Errorf("Expected error for server %s, got nil", invalidServerAddress) } } @@ -207,6 +214,6 @@ func TestList(t *testing.T) { } if username, exists := auths[validServerAddress]; !exists || username != validUsername { - t.Fatalf("auths[%s] returned %s, %t; expected %s, %t", validServerAddress, username, exists, validUsername, true) + t.Errorf("auths[%s] returned %s, %t; expected %s, %t", validServerAddress, username, exists, validUsername, true) } } diff --git a/credentials/credentials_test.go b/credentials/credentials_test.go index e56281e..587d180 100644 --- a/credentials/credentials_test.go +++ b/credentials/credentials_test.go @@ -42,7 +42,7 @@ func (m *memoryStore) List() (map[string]string, error) { } func TestStore(t *testing.T) { - serverURL := "https://index.docker.io/v1/" + const serverURL = "https://index.docker.io/v1/" creds := &Credentials{ ServerURL: serverURL, Username: "foo", @@ -65,11 +65,11 @@ func TestStore(t *testing.T) { } if c.Username != "foo" { - t.Fatalf("expected username foo, got %s\n", c.Username) + t.Errorf("expected username foo, got %s\n", c.Username) } if c.Secret != "bar" { - t.Fatalf("expected username bar, got %s\n", c.Secret) + t.Errorf("expected username bar, got %s\n", c.Secret) } } @@ -89,7 +89,7 @@ func TestStoreMissingServerURL(t *testing.T) { h := newMemoryStore() if err := Store(h, in); IsCredentialsMissingServerURL(err) == false { - t.Fatal(err) + t.Error(err) } } @@ -109,12 +109,12 @@ func TestStoreMissingUsername(t *testing.T) { h := newMemoryStore() if err := Store(h, in); IsCredentialsMissingUsername(err) == false { - t.Fatal(err) + t.Error(err) } } func TestGet(t *testing.T) { - serverURL := "https://index.docker.io/v1/" + const serverURL = "https://index.docker.io/v1/" creds := &Credentials{ ServerURL: serverURL, Username: "foo", @@ -147,16 +147,16 @@ func TestGet(t *testing.T) { } if c.Username != "foo" { - t.Fatalf("expected username foo, got %s\n", c.Username) + t.Errorf("expected username foo, got %s\n", c.Username) } if c.Secret != "bar" { - t.Fatalf("expected username bar, got %s\n", c.Secret) + t.Errorf("expected username bar, got %s\n", c.Secret) } } func TestGetMissingServerURL(t *testing.T) { - serverURL := "https://index.docker.io/v1/" + const serverURL = "https://index.docker.io/v1/" creds := &Credentials{ ServerURL: serverURL, Username: "foo", @@ -177,12 +177,12 @@ func TestGetMissingServerURL(t *testing.T) { w := new(bytes.Buffer) if err := Get(h, buf, w); IsCredentialsMissingServerURL(err) == false { - t.Fatal(err) + t.Error(err) } } func TestErase(t *testing.T) { - serverURL := "https://index.docker.io/v1/" + const serverURL = "https://index.docker.io/v1/" creds := &Credentials{ ServerURL: serverURL, Username: "foo", @@ -206,12 +206,12 @@ func TestErase(t *testing.T) { w := new(bytes.Buffer) if err := Get(h, buf, w); err == nil { - t.Fatal("expected error getting missing creds, got empty") + t.Error("expected error getting missing creds, got empty") } } func TestEraseMissingServerURL(t *testing.T) { - serverURL := "https://index.docker.io/v1/" + const serverURL = "https://index.docker.io/v1/" creds := &Credentials{ ServerURL: serverURL, Username: "foo", @@ -244,6 +244,6 @@ func TestList(t *testing.T) { } // testing that there is an output if out.Len() == 0 { - t.Fatalf("expected output in the writer, got %d", 0) + t.Error("expected output in the writer, got 0") } } diff --git a/osxkeychain/osxkeychain_darwin_test.go b/osxkeychain/osxkeychain_darwin_test.go index b5098ef..425f314 100644 --- a/osxkeychain/osxkeychain_darwin_test.go +++ b/osxkeychain/osxkeychain_darwin_test.go @@ -60,41 +60,57 @@ func TestOSXKeychainHelper(t *testing.T) { // through variations on the URL func TestOSXKeychainHelperRetrieveAliases(t *testing.T) { tests := []struct { + doc string storeURL string readURL string }{ - // stored with port, retrieved without - {"https://foobar.docker.io:2376", "https://foobar.docker.io"}, - - // stored as https, retrieved without scheme - {"https://foobar.docker.io:2376", "foobar.docker.io"}, - - // stored with path, retrieved without - {"https://foobar.docker.io:1234/one/two", "https://foobar.docker.io:1234"}, + { + doc: "stored with port, retrieved without", + storeURL: "https://foobar.docker.io:2376", + readURL: "https://foobar.docker.io", + }, + { + doc: "stored as https, retrieved without scheme", + storeURL: "https://foobar.docker.io:2376", + readURL: "foobar.docker.io", + }, + { + doc: "stored with path, retrieved without", + storeURL: "https://foobar.docker.io:1234/one/two", + readURL: "https://foobar.docker.io:1234", + }, } helper := Osxkeychain{} - defer func() { - for _, te := range tests { - helper.Delete(te.storeURL) + t.Cleanup(func() { + for _, tc := range tests { + if err := helper.Delete(tc.storeURL); err != nil && !credentials.IsErrCredentialsNotFound(err) { + t.Errorf("cleanup: failed to delete '%s': %v", tc.storeURL, err) + } } - }() + }) // Clean store before testing. - for _, te := range tests { - helper.Delete(te.storeURL) + for _, tc := range tests { + if err := helper.Delete(tc.storeURL); err != nil && !credentials.IsErrCredentialsNotFound(err) { + t.Errorf("prepare: failed to delete '%s': %v", tc.storeURL, err) + } } - for _, te := range tests { - c := &credentials.Credentials{ServerURL: te.storeURL, Username: "hello", Secret: "world"} - if err := helper.Add(c); err != nil { - t.Errorf("Error: failed to store secret for URL %q: %s", te.storeURL, err) - continue - } - if _, _, err := helper.Get(te.readURL); err != nil { - t.Errorf("Error: failed to read secret for URL %q using %q", te.storeURL, te.readURL) - } - helper.Delete(te.storeURL) + for _, tc := range tests { + tc := tc + t.Run(tc.doc, func(t *testing.T) { + c := &credentials.Credentials{ServerURL: tc.storeURL, Username: "hello", Secret: "world"} + if err := helper.Add(c); err != nil { + t.Fatalf("Error: failed to store secret for URL %q: %s", tc.storeURL, err) + } + if _, _, err := helper.Get(tc.readURL); err != nil { + t.Errorf("Error: failed to read secret for URL %q using %q", tc.storeURL, tc.readURL) + } + if err := helper.Delete(tc.storeURL); err != nil { + t.Error(err) + } + }) } } @@ -102,50 +118,74 @@ func TestOSXKeychainHelperRetrieveAliases(t *testing.T) { // returned. func TestOSXKeychainHelperRetrieveStrict(t *testing.T) { tests := []struct { + doc string storeURL string readURL string }{ - // stored as https, retrieved using http - {"https://foobar.docker.io:2376", "http://foobar.docker.io:2376"}, - - // stored as http, retrieved using https - {"http://foobar.docker.io:2376", "https://foobar.docker.io:2376"}, - - // same: stored as http, retrieved without a scheme specified (hence, using the default https://) - {"http://foobar.docker.io", "foobar.docker.io:5678"}, - - // non-matching ports - {"https://foobar.docker.io:1234", "https://foobar.docker.io:5678"}, - - // non-matching ports TODO is this desired behavior? The other way round does work - //{"https://foobar.docker.io", "https://foobar.docker.io:5678"}, - - // non-matching paths - {"https://foobar.docker.io:1234/one/two", "https://foobar.docker.io:1234/five/six"}, + { + doc: "stored as https, retrieved using http", + storeURL: "https://foobar.docker.io:2376", + readURL: "http://foobar.docker.io:2376", + }, + { + doc: "stored as http, retrieved using https", + storeURL: "http://foobar.docker.io:2376", + readURL: "https://foobar.docker.io:2376", + }, + { + // stored as http, retrieved without a scheme specified (hence, using the default https://) + doc: "stored as http, retrieved without scheme", + storeURL: "http://foobar.docker.io", + readURL: "foobar.docker.io:5678", + }, + { + doc: "non-matching ports", + storeURL: "https://foobar.docker.io:1234", + readURL: "https://foobar.docker.io:5678", + }, + // TODO: is this desired behavior? The other way round does work + // { + // doc: "non-matching ports (stored without port)", + // storeURL: "https://foobar.docker.io", + // readURL: "https://foobar.docker.io:5678", + // }, + { + doc: "non-matching paths", + storeURL: "https://foobar.docker.io:1234/one/two", + readURL: "https://foobar.docker.io:1234/five/six", + }, } helper := Osxkeychain{} - defer func() { - for _, te := range tests { - helper.Delete(te.storeURL) + t.Cleanup(func() { + for _, tc := range tests { + if err := helper.Delete(tc.storeURL); err != nil && !credentials.IsErrCredentialsNotFound(err) { + t.Errorf("cleanup: failed to delete '%s': %v", tc.storeURL, err) + } } - }() + }) // Clean store before testing. - for _, te := range tests { - helper.Delete(te.storeURL) + for _, tc := range tests { + if err := helper.Delete(tc.storeURL); err != nil && !credentials.IsErrCredentialsNotFound(err) { + t.Errorf("prepare: failed to delete '%s': %v", tc.storeURL, err) + } } - for _, te := range tests { - c := &credentials.Credentials{ServerURL: te.storeURL, Username: "hello", Secret: "world"} - if err := helper.Add(c); err != nil { - t.Errorf("Error: failed to store secret for URL %q: %s", te.storeURL, err) - continue - } - if _, _, err := helper.Get(te.readURL); err == nil { - t.Errorf("Error: managed to read secret for URL %q using %q, but should not be able to", te.storeURL, te.readURL) - } - helper.Delete(te.storeURL) + for _, tc := range tests { + tc := tc + t.Run(tc.doc, func(t *testing.T) { + c := &credentials.Credentials{ServerURL: tc.storeURL, Username: "hello", Secret: "world"} + if err := helper.Add(c); err != nil { + t.Fatalf("Error: failed to store secret for URL %q: %s", tc.storeURL, err) + } + if _, _, err := helper.Get(tc.readURL); err == nil { + t.Errorf("Error: managed to read secret for URL %q using %q, but should not be able to", tc.storeURL, tc.readURL) + } + if err := helper.Delete(tc.storeURL); err != nil { + t.Error(err) + } + }) } } @@ -166,41 +206,46 @@ func TestOSXKeychainHelperStoreRetrieve(t *testing.T) { } helper := Osxkeychain{} - defer func() { - for _, te := range tests { - helper.Delete(te.url) + t.Cleanup(func() { + for _, tc := range tests { + if err := helper.Delete(tc.url); err != nil && !credentials.IsErrCredentialsNotFound(err) { + t.Errorf("cleanup: failed to delete '%s': %v", tc.url, err) + } } - }() + }) // Clean store before testing. - for _, te := range tests { - helper.Delete(te.url) + for _, tc := range tests { + if err := helper.Delete(tc.url); err != nil && !credentials.IsErrCredentialsNotFound(err) { + t.Errorf("prepare: failed to delete '%s': %v", tc.url, err) + } } // Note that we don't delete between individual tests here, to verify that // subsequent stores/overwrites don't affect storing / retrieving secrets. - for i, te := range tests { - c := &credentials.Credentials{ - ServerURL: te.url, - Username: fmt.Sprintf("user-%d", i), - Secret: fmt.Sprintf("secret-%d", i), - } + for i, tc := range tests { + tc := tc + t.Run(tc.url, func(t *testing.T) { + c := &credentials.Credentials{ + ServerURL: tc.url, + Username: fmt.Sprintf("user-%d", i), + Secret: fmt.Sprintf("secret-%d", i), + } - if err := helper.Add(c); err != nil { - t.Errorf("Error: failed to store secret for URL: %s: %s", te.url, err) - continue - } - user, secret, err := helper.Get(te.url) - if err != nil { - t.Errorf("Error: failed to read secret for URL %q: %s", te.url, err) - continue - } - if user != c.Username { - t.Errorf("Error: expected username %s, got username %s for URL: %s", c.Username, user, te.url) - } - if secret != c.Secret { - t.Errorf("Error: expected secret %s, got secret %s for URL: %s", c.Secret, secret, te.url) - } + if err := helper.Add(c); err != nil { + t.Fatalf("Error: failed to store secret for URL: %s: %s", tc.url, err) + } + user, secret, err := helper.Get(tc.url) + if err != nil { + t.Fatalf("Error: failed to read secret for URL %q: %s", tc.url, err) + } + if user != c.Username { + t.Errorf("Error: expected username %s, got username %s for URL: %s", c.Username, user, tc.url) + } + if secret != c.Secret { + t.Errorf("Error: expected secret %s, got secret %s for URL: %s", c.Secret, secret, tc.url) + } + }) } } diff --git a/registryurl/parse_test.go b/registryurl/parse_test.go index 89feaa7..b5c161d 100644 --- a/registryurl/parse_test.go +++ b/registryurl/parse_test.go @@ -13,34 +13,61 @@ func TestHelperParseURL(t *testing.T) { expectedURL string err error }{ - {url: "foobar.docker.io", expectedURL: "//foobar.docker.io"}, - {url: "foobar.docker.io:2376", expectedURL: "//foobar.docker.io:2376"}, - {url: "//foobar.docker.io:2376", expectedURL: "//foobar.docker.io:2376"}, - {url: "http://foobar.docker.io:2376", expectedURL: "http://foobar.docker.io:2376"}, - {url: "https://foobar.docker.io:2376", expectedURL: "https://foobar.docker.io:2376"}, - {url: "https://foobar.docker.io:2376/some/path", expectedURL: "https://foobar.docker.io:2376/some/path"}, - {url: "https://foobar.docker.io:2376/some/other/path?foo=bar", expectedURL: "https://foobar.docker.io:2376/some/other/path"}, - {url: "/foobar.docker.io", err: errors.New("no hostname in URL")}, - {url: "ftp://foobar.docker.io:2376", err: errors.New("unsupported scheme: ftp")}, + { + url: "foobar.docker.io", + expectedURL: "//foobar.docker.io", + }, + { + url: "foobar.docker.io:2376", + expectedURL: "//foobar.docker.io:2376", + }, + { + url: "//foobar.docker.io:2376", + expectedURL: "//foobar.docker.io:2376", + }, + { + url: "http://foobar.docker.io:2376", + expectedURL: "http://foobar.docker.io:2376", + }, + { + url: "https://foobar.docker.io:2376", + expectedURL: "https://foobar.docker.io:2376", + }, + { + url: "https://foobar.docker.io:2376/some/path", + expectedURL: "https://foobar.docker.io:2376/some/path", + }, + { + url: "https://foobar.docker.io:2376/some/other/path?foo=bar", + expectedURL: "https://foobar.docker.io:2376/some/other/path", + }, + { + url: "/foobar.docker.io", + err: errors.New("no hostname in URL"), + }, + { + url: "ftp://foobar.docker.io:2376", + err: errors.New("unsupported scheme: ftp"), + }, } - for _, te := range tests { - u, err := Parse(te.url) + for _, tc := range tests { + tc := tc + t.Run(tc.url, func(t *testing.T) { + u, err := Parse(tc.url) - if te.err == nil && err != nil { - t.Errorf("Error: failed to parse URL %q: %s", te.url, err) - continue - } - if te.err != nil && err == nil { - t.Errorf("Error: expected error %q, got none when parsing URL %q", te.err, te.url) - continue - } - if te.err != nil && err.Error() != te.err.Error() { - t.Errorf("Error: expected error %q, got %q when parsing URL %q", te.err, err, te.url) - continue - } - if u != nil && u.String() != te.expectedURL { - t.Errorf("Error: expected URL: %q, but got %q for URL: %q", te.expectedURL, u.String(), te.url) - } + if tc.err == nil && err != nil { + t.Fatalf("Error: failed to parse URL %q: %s", tc.url, err) + } + if tc.err != nil && err == nil { + t.Fatalf("Error: expected error %q, got none when parsing URL %q", tc.err, tc.url) + } + if tc.err != nil && err.Error() != tc.err.Error() { + t.Fatalf("Error: expected error %q, got %q when parsing URL %q", tc.err, err, tc.url) + } + if u != nil && u.String() != tc.expectedURL { + t.Errorf("Error: expected URL: %q, but got %q for URL: %q", tc.expectedURL, u.String(), tc.url) + } + }) } } diff --git a/wincred/wincred_windows_test.go b/wincred/wincred_windows_test.go index 68d293a..e6010fe 100644 --- a/wincred/wincred_windows_test.go +++ b/wincred/wincred_windows_test.go @@ -91,41 +91,57 @@ func TestWinCredHelper(t *testing.T) { // through variations on the URL func TestWinCredHelperRetrieveAliases(t *testing.T) { tests := []struct { + doc string storeURL string readURL string }{ - // stored with port, retrieved without - {"https://foobar.docker.io:2376", "https://foobar.docker.io"}, - - // stored as https, retrieved without scheme - {"https://foobar.docker.io", "foobar.docker.io"}, - - // stored with path, retrieved without - {"https://foobar.docker.io/one/two", "https://foobar.docker.io"}, + { + doc: "stored with port, retrieved without", + storeURL: "https://foobar.docker.io:2376", + readURL: "https://foobar.docker.io", + }, + { + doc: "stored as https, retrieved without scheme", + storeURL: "https://foobar.docker.io", + readURL: "foobar.docker.io", + }, + { + doc: "stored with path, retrieved without", + storeURL: "https://foobar.docker.io/one/two", + readURL: "https://foobar.docker.io", + }, } helper := Wincred{} - defer func() { - for _, te := range tests { - helper.Delete(te.storeURL) + t.Cleanup(func() { + for _, tc := range tests { + if err := helper.Delete(tc.storeURL); err != nil && !credentials.IsErrCredentialsNotFound(err) { + t.Errorf("cleanup: failed to delete '%s': %v", tc.storeURL, err) + } } - }() + }) // Clean store before testing. - for _, te := range tests { - helper.Delete(te.storeURL) + for _, tc := range tests { + if err := helper.Delete(tc.storeURL); err != nil && !credentials.IsErrCredentialsNotFound(err) { + t.Errorf("prepare: failed to delete '%s': %v", tc.storeURL, err) + } } - for _, te := range tests { - c := &credentials.Credentials{ServerURL: te.storeURL, Username: "hello", Secret: "world"} - if err := helper.Add(c); err != nil { - t.Errorf("Error: failed to store secret for URL %q: %s", te.storeURL, err) - continue - } - if _, _, err := helper.Get(te.readURL); err != nil { - t.Errorf("Error: failed to read secret for URL %q using %q", te.storeURL, te.readURL) - } - helper.Delete(te.storeURL) + for _, tc := range tests { + tc := tc + t.Run(tc.doc, func(t *testing.T) { + c := &credentials.Credentials{ServerURL: tc.storeURL, Username: "hello", Secret: "world"} + if err := helper.Add(c); err != nil { + t.Fatalf("Error: failed to store secret for URL %q: %s", tc.storeURL, err) + } + if _, _, err := helper.Get(tc.readURL); err != nil { + t.Errorf("Error: failed to read secret for URL %q using %q", tc.storeURL, tc.readURL) + } + if err := helper.Delete(tc.storeURL); err != nil { + t.Error(err) + } + }) } } @@ -133,50 +149,74 @@ func TestWinCredHelperRetrieveAliases(t *testing.T) { // returned. func TestWinCredHelperRetrieveStrict(t *testing.T) { tests := []struct { + doc string storeURL string readURL string }{ - // stored as https, retrieved using http - {"https://foobar.docker.io:2376", "http://foobar.docker.io:2376"}, - - // stored as http, retrieved using https - {"http://foobar.docker.io:2376", "https://foobar.docker.io:2376"}, - - // same: stored as http, retrieved without a scheme specified (hence, using the default https://) - {"http://foobar.docker.io", "foobar.docker.io:5678"}, - - // non-matching ports - {"https://foobar.docker.io:1234", "https://foobar.docker.io:5678"}, - - // non-matching ports TODO is this desired behavior? The other way round does work - //{"https://foobar.docker.io", "https://foobar.docker.io:5678"}, - - // non-matching paths - {"https://foobar.docker.io:1234/one/two", "https://foobar.docker.io:1234/five/six"}, + { + doc: "stored as https, retrieved using http", + storeURL: "https://foobar.docker.io:2376", + readURL: "http://foobar.docker.io:2376", + }, + { + doc: "stored as http, retrieved using https", + storeURL: "http://foobar.docker.io:2376", + readURL: "https://foobar.docker.io:2376", + }, + { + // stored as http, retrieved without a scheme specified (hence, using the default https://) + doc: "stored as http, retrieved without scheme", + storeURL: "http://foobar.docker.io", + readURL: "foobar.docker.io:5678", + }, + { + doc: "non-matching ports", + storeURL: "https://foobar.docker.io:1234", + readURL: "https://foobar.docker.io:5678", + }, + // TODO: is this desired behavior? The other way round does work + // { + // doc: "non-matching ports (stored without port)", + // storeURL: "https://foobar.docker.io", + // readURL: "https://foobar.docker.io:5678", + // }, + { + doc: "non-matching paths", + storeURL: "https://foobar.docker.io:1234/one/two", + readURL: "https://foobar.docker.io:1234/five/six", + }, } helper := Wincred{} - defer func() { - for _, te := range tests { - helper.Delete(te.storeURL) + t.Cleanup(func() { + for _, tc := range tests { + if err := helper.Delete(tc.storeURL); err != nil && !credentials.IsErrCredentialsNotFound(err) { + t.Errorf("cleanup: failed to delete '%s': %v", tc.storeURL, err) + } } - }() + }) // Clean store before testing. - for _, te := range tests { - helper.Delete(te.storeURL) + for _, tc := range tests { + if err := helper.Delete(tc.storeURL); err != nil && !credentials.IsErrCredentialsNotFound(err) { + t.Errorf("prepare: failed to delete '%s': %v", tc.storeURL, err) + } } - for _, te := range tests { - c := &credentials.Credentials{ServerURL: te.storeURL, Username: "hello", Secret: "world"} - if err := helper.Add(c); err != nil { - t.Errorf("Error: failed to store secret for URL %q: %s", te.storeURL, err) - continue - } - if _, _, err := helper.Get(te.readURL); err == nil { - t.Errorf("Error: managed to read secret for URL %q using %q, but should not be able to", te.storeURL, te.readURL) - } - helper.Delete(te.storeURL) + for _, tc := range tests { + tc := tc + t.Run(tc.doc, func(t *testing.T) { + c := &credentials.Credentials{ServerURL: tc.storeURL, Username: "hello", Secret: "world"} + if err := helper.Add(c); err != nil { + t.Fatalf("Error: failed to store secret for URL %q: %s", tc.storeURL, err) + } + if _, _, err := helper.Get(tc.readURL); err == nil { + t.Errorf("Error: managed to read secret for URL %q using %q, but should not be able to", tc.storeURL, tc.readURL) + } + if err := helper.Delete(tc.storeURL); err != nil { + t.Error(err) + } + }) } } @@ -197,41 +237,46 @@ func TestWinCredHelperStoreRetrieve(t *testing.T) { } helper := Wincred{} - defer func() { - for _, te := range tests { - helper.Delete(te.url) + t.Cleanup(func() { + for _, tc := range tests { + if err := helper.Delete(tc.url); err != nil && !credentials.IsErrCredentialsNotFound(err) { + t.Errorf("cleanup: failed to delete '%s': %v", tc.url, err) + } } - }() + }) // Clean store before testing. - for _, te := range tests { - helper.Delete(te.url) + for _, tc := range tests { + if err := helper.Delete(tc.url); err != nil && !credentials.IsErrCredentialsNotFound(err) { + t.Errorf("prepare: failed to delete '%s': %v", tc.url, err) + } } // Note that we don't delete between individual tests here, to verify that // subsequent stores/overwrites don't affect storing / retrieving secrets. - for i, te := range tests { - c := &credentials.Credentials{ - ServerURL: te.url, - Username: fmt.Sprintf("user-%d", i), - Secret: fmt.Sprintf("secret-%d", i), - } + for i, tc := range tests { + tc := tc + t.Run(tc.url, func(t *testing.T) { + c := &credentials.Credentials{ + ServerURL: tc.url, + Username: fmt.Sprintf("user-%d", i), + Secret: fmt.Sprintf("secret-%d", i), + } - if err := helper.Add(c); err != nil { - t.Errorf("Error: failed to store secret for URL: %s: %s", te.url, err) - continue - } - user, secret, err := helper.Get(te.url) - if err != nil { - t.Errorf("Error: failed to read secret for URL %q: %s", te.url, err) - continue - } - if user != c.Username { - t.Errorf("Error: expected username %s, got username %s for URL: %s", c.Username, user, te.url) - } - if secret != c.Secret { - t.Errorf("Error: expected secret %s, got secret %s for URL: %s", c.Secret, secret, te.url) - } + if err := helper.Add(c); err != nil { + t.Fatalf("Error: failed to store secret for URL: %s: %s", tc.url, err) + } + user, secret, err := helper.Get(tc.url) + if err != nil { + t.Fatalf("Error: failed to read secret for URL %q: %s", tc.url, err) + } + if user != c.Username { + t.Errorf("Error: expected username %s, got username %s for URL: %s", c.Username, user, tc.url) + } + if secret != c.Secret { + t.Errorf("Error: expected secret %s, got secret %s for URL: %s", c.Secret, secret, tc.url) + } + }) } }