refactor: Use t.Fatal instead of t.Error (#3340)

This commit changes instances of t.Error followed by an immediate return
with t.Fatal, where valid.

Up until today I thought that t.Fatal didn't run defers.

I was [wrong](https://github.com/tsenart/vegeta/pull/386#discussion_r273691705)
as @dominikh pointed out in a PR in vegeta.
This commit is contained in:
Tomás Senart 2019-04-10 14:03:10 +02:00 committed by GitHub
parent b1bb4578f8
commit 079a5c050d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 22 additions and 48 deletions

View File

@ -301,21 +301,18 @@ func testEnabledStateDeprecationMigration(store repos.Store) func(*testing.T) {
t.Run(tc.name, transact(ctx, store, func(t testing.TB, tx repos.Store) {
if err := tx.UpsertRepos(ctx, tc.stored.Clone()...); err != nil {
t.Errorf("failed to prepare store: %v", err)
return
t.Fatalf("failed to prepare store: %v", err)
}
err := repos.EnabledStateDeprecationMigration(tc.sourcer, clock.Now).Run(ctx, tx)
if have, want := fmt.Sprint(err), tc.err; have != want {
t.Errorf("error:\nhave: %v\nwant: %v", have, want)
return
t.Fatalf("error:\nhave: %v\nwant: %v", have, want)
}
if tc.svcs != nil {
svcs, err := tx.ListExternalServices(ctx, repos.StoreListExternalServicesArgs{})
if err != nil {
t.Error(err)
return
t.Fatal(err)
}
tc.svcs(t, svcs)
}
@ -323,8 +320,7 @@ func testEnabledStateDeprecationMigration(store repos.Store) func(*testing.T) {
if tc.repos != nil {
rs, err := tx.ListRepos(ctx, repos.StoreListReposArgs{})
if err != nil {
t.Error(err)
return
t.Fatal(err)
}
tc.repos(t, rs)
}
@ -453,8 +449,7 @@ func testGithubSetDefaultRepositoryQueryMigration(store repos.Store) func(*testi
t.Run(tc.name, transact(ctx, store, func(t testing.TB, tx repos.Store) {
if err := tx.UpsertExternalServices(ctx, tc.stored.Clone()...); err != nil {
t.Errorf("failed to prepare store: %v", err)
return
t.Fatalf("failed to prepare store: %v", err)
}
err := repos.GithubSetDefaultRepositoryQueryMigration(clock.Now).Run(ctx, tx)
@ -464,8 +459,7 @@ func testGithubSetDefaultRepositoryQueryMigration(store repos.Store) func(*testi
es, err := tx.ListExternalServices(ctx, repos.StoreListExternalServicesArgs{})
if err != nil {
t.Error(err)
return
t.Fatal(err)
}
if tc.assert != nil {
@ -571,8 +565,7 @@ func testGitLabSetDefaultProjectQueryMigration(store repos.Store) func(*testing.
t.Run(tc.name, transact(ctx, store, func(t testing.TB, tx repos.Store) {
if err := tx.UpsertExternalServices(ctx, tc.stored.Clone()...); err != nil {
t.Errorf("failed to prepare store: %v", err)
return
t.Fatalf("failed to prepare store: %v", err)
}
err := repos.GitLabSetDefaultProjectQueryMigration(clock.Now).Run(ctx, tx)
@ -582,8 +575,7 @@ func testGitLabSetDefaultProjectQueryMigration(store repos.Store) func(*testing.
es, err := tx.ListExternalServices(ctx, repos.StoreListExternalServicesArgs{})
if err != nil {
t.Error(err)
return
t.Fatal(err)
}
if tc.assert != nil {
@ -690,8 +682,7 @@ func testBitbucketServerSetDefaultRepositoryQueryMigration(store repos.Store) fu
t.Run(tc.name, transact(ctx, store, func(t testing.TB, tx repos.Store) {
if err := tx.UpsertExternalServices(ctx, tc.stored.Clone()...); err != nil {
t.Errorf("failed to prepare store: %v", err)
return
t.Fatalf("failed to prepare store: %v", err)
}
err := repos.BitbucketServerSetDefaultRepositoryQueryMigration(clock.Now).Run(ctx, tx)
@ -701,8 +692,7 @@ func testBitbucketServerSetDefaultRepositoryQueryMigration(store repos.Store) fu
es, err := tx.ListExternalServices(ctx, repos.StoreListExternalServicesArgs{})
if err != nil {
t.Error(err)
return
t.Fatal(err)
}
if tc.assert != nil {

View File

@ -369,8 +369,7 @@ func TestSources_ListRepos(t *testing.T) {
srcs, err := NewSourcer(cf)(tc.svcs...)
if err != nil {
t.Error(err)
return // Let defers run
t.Fatal(err)
}
ctx := tc.ctx

View File

@ -118,8 +118,7 @@ func testStoreListExternalServices(store repos.Store) func(*testing.T) {
t.Run(tc.name, transact(ctx, store, func(t testing.TB, tx repos.Store) {
if err := tx.UpsertExternalServices(ctx, tc.stored.Clone()...); err != nil {
t.Errorf("failed to setup store: %v", err)
return
t.Fatalf("failed to setup store: %v", err)
}
es, err := tx.ListExternalServices(ctx, repos.StoreListExternalServicesArgs{
@ -188,8 +187,7 @@ func testStoreUpsertExternalServices(store repos.Store) func(*testing.T) {
want := mkExternalServices(512, svcs...)
if err := tx.UpsertExternalServices(ctx, want...); err != nil {
t.Errorf("UpsertExternalServices error: %s", err)
return
t.Fatalf("UpsertExternalServices error: %s", err)
}
for _, e := range want {
@ -206,13 +204,11 @@ func testStoreUpsertExternalServices(store repos.Store) func(*testing.T) {
})
if err != nil {
t.Errorf("ListExternalServices error: %s", err)
return
t.Fatalf("ListExternalServices error: %s", err)
}
if diff := pretty.Compare(have, want); diff != "" {
t.Errorf("ListExternalServices:\n%s", diff)
return
t.Fatalf("ListExternalServices:\n%s", diff)
}
now := clock.Now()
@ -338,8 +334,7 @@ func testStoreUpsertRepos(store repos.Store) func(*testing.T) {
want := mkRepos(512, repositories...)
if err := tx.UpsertRepos(ctx, want...); err != nil {
t.Errorf("UpsertRepos error: %s", err)
return
t.Fatalf("UpsertRepos error: %s", err)
}
sort.Sort(want)
@ -349,13 +344,11 @@ func testStoreUpsertRepos(store repos.Store) func(*testing.T) {
})
if err != nil {
t.Errorf("ListRepos error: %s", err)
return
t.Fatalf("ListRepos error: %s", err)
}
if diff := pretty.Compare(have, want); diff != "" {
t.Errorf("ListRepos:\n%s", diff)
return
t.Fatalf("ListRepos:\n%s", diff)
}
suffix := "-updated"
@ -563,8 +556,7 @@ func testStoreListRepos(store repos.Store) func(*testing.T) {
t.Run(tc.name, transact(ctx, store, func(t testing.TB, tx repos.Store) {
stored := tc.stored.Clone()
if err := tx.UpsertRepos(ctx, stored...); err != nil {
t.Errorf("failed to setup store: %v", err)
return
t.Fatalf("failed to setup store: %v", err)
}
var args repos.StoreListReposArgs
@ -643,9 +635,7 @@ func transact(ctx context.Context, s repos.Store, test func(testing.TB, repos.St
if ok {
txstore, err := tr.Transact(ctx)
if err != nil {
// NOTE: We use t.Errorf instead of t.Fatalf in order to run defers.
t.Errorf("failed to start transaction: %v", err)
return
t.Fatalf("failed to start transaction: %v", err)
}
defer txstore.Done(&errRollback)
s = &noopTxStore{TB: t, Store: txstore}

View File

@ -693,8 +693,7 @@ func TestRepoLookup_syncer(t *testing.T) {
t.Fatal(err)
}
if diff := pretty.Compare(result, want); diff != "" {
t.Errorf("ListRepos:\n%s", diff)
return
t.Fatalf("ListRepos:\n%s", diff)
}
})
}

View File

@ -78,7 +78,6 @@ func TestPrepareZip(t *testing.T) {
_, err := s.prepareZip(context.Background(), wantRepo, wantCommit)
if err != nil {
t.Fatal("expected prepareZip to succeed:", err)
return
}
}
@ -99,7 +98,6 @@ func tmpStore(t *testing.T) (*Store, func()) {
d, err := ioutil.TempDir("", "search_test")
if err != nil {
t.Fatal(err)
return nil, nil
}
return &Store{
Path: d,
@ -112,7 +110,6 @@ func emptyTar(t *testing.T) io.ReadCloser {
err := w.Close()
if err != nil {
t.Fatal(err)
return nil
}
return ioutil.NopCloser(bytes.NewReader(buf.Bytes()))
}

View File

@ -198,8 +198,7 @@ func Test_GitLab_FetchAccount(t *testing.T) {
t.Run(c.description, func(t *testing.T) {
acct, err := authzProvider.FetchAccount(ctx, c.user, c.current)
if err != nil {
t.Errorf("unexpected error: %v", err)
return
t.Fatalf("unexpected error: %v", err)
}
// ignore AccountData field in comparison
if acct != nil {