From b0b6c962dcbfac06c932e5f5133d97f17606537b Mon Sep 17 00:00:00 2001 From: Anna Song Date: Wed, 1 Jun 2022 17:46:30 -0400 Subject: [PATCH] Replace lengthy t.Fatalf with require --- api/loader/fileloader_test.go | 364 ++++++++++++---------------------- 1 file changed, 130 insertions(+), 234 deletions(-) diff --git a/api/loader/fileloader_test.go b/api/loader/fileloader_test.go index 0f9d5762a6..c83dc816ce 100644 --- a/api/loader/fileloader_test.go +++ b/api/loader/fileloader_test.go @@ -14,6 +14,8 @@ import ( "strings" "testing" + "github.com/stretchr/testify/require" + "sigs.k8s.io/kustomize/api/ifc" "sigs.k8s.io/kustomize/api/internal/git" "sigs.k8s.io/kustomize/api/konfig" @@ -57,124 +59,94 @@ func makeLoader() *fileLoader { } func TestLoaderLoad(t *testing.T) { + require := require.New(t) + l1 := makeLoader() - if "/" != l1.Root() { - t.Fatalf("incorrect root: '%s'\n", l1.Root()) - } + require.Equal("/", l1.Root()) + for _, x := range testCases { b, err := l1.Load(x.path) - if err != nil { - t.Fatalf("unexpected load error: %v", err) - } + require.NoError(err) + if !reflect.DeepEqual([]byte(x.expectedContent), b) { t.Fatalf("in load expected %s, but got %s", x.expectedContent, b) } } l2, err := l1.New("foo/project") - if err != nil { - t.Fatalf("unexpected err: %v\n", err) - } - if "/foo/project" != l2.Root() { - t.Fatalf("incorrect root: %s\n", l2.Root()) - } + require.NoError(err) + require.Equal("/foo/project", l2.Root()) + for _, x := range testCases { b, err := l2.Load(strings.TrimPrefix(x.path, "foo/project/")) - if err != nil { - t.Fatalf("unexpected load error %v", err) - } + require.NoError(err) + if !reflect.DeepEqual([]byte(x.expectedContent), b) { t.Fatalf("in load expected %s, but got %s", x.expectedContent, b) } } l2, err = l1.New("foo/project/") // Assure trailing slash stripped - if err != nil { - t.Fatalf("unexpected err: %v\n", err) - } - if "/foo/project" != l2.Root() { - t.Fatalf("incorrect root: %s\n", l2.Root()) - } + require.NoError(err) + require.Equal("/foo/project", l2.Root()) } func TestLoaderNewSubDir(t *testing.T) { + require := require.New(t) + l1, err := makeLoader().New("foo/project") - if err != nil { - t.Fatalf("unexpected err: %v\n", err) - } + require.NoError(err) + l2, err := l1.New("subdir1") - if err != nil { - t.Fatalf("unexpected err: %v\n", err) - } - if "/foo/project/subdir1" != l2.Root() { - t.Fatalf("incorrect root: %s\n", l2.Root()) - } + require.NoError(err) + require.Equal("/foo/project/subdir1", l2.Root()) + x := testCases[1] b, err := l2.Load("fileB.yaml") - if err != nil { - t.Fatalf("unexpected load error %v", err) - } + require.NoError(err) + if !reflect.DeepEqual([]byte(x.expectedContent), b) { t.Fatalf("in load expected %s, but got %s", x.expectedContent, b) } } func TestLoaderBadRelative(t *testing.T) { + require := require.New(t) + l1, err := makeLoader().New("foo/project/subdir1") - if err != nil { - t.Fatalf("unexpected err: %v\n", err) - } - if "/foo/project/subdir1" != l1.Root() { - t.Fatalf("incorrect root: %s\n", l1.Root()) - } + require.NoError(err) + require.Equal("/foo/project/subdir1", l1.Root()) // Cannot cd into a file. l2, err := l1.New("fileB.yaml") - if err == nil { - t.Fatalf("expected err, but got root %s", l2.Root()) - } + require.Error(err) // It's not okay to stay at the same place. l2, err = l1.New(filesys.SelfDir) - if err == nil { - t.Fatalf("expected err, but got root %s", l2.Root()) - } + require.Error(err) // It's not okay to go up and back down into same place. l2, err = l1.New("../subdir1") - if err == nil { - t.Fatalf("expected err, but got root %s", l2.Root()) - } + require.Error(err) // It's not okay to go up via a relative path. l2, err = l1.New("..") - if err == nil { - t.Fatalf("expected err, but got root %s", l2.Root()) - } + require.Error(err) // It's not okay to go up via an absolute path. l2, err = l1.New("/foo/project") - if err == nil { - t.Fatalf("expected err, but got root %s", l2.Root()) - } + require.Error(err) // It's not okay to go to the root. l2, err = l1.New("/") - if err == nil { - t.Fatalf("expected err, but got root %s", l2.Root()) - } + require.Error(err) // It's okay to go up and down to a sibling. l2, err = l1.New("../subdir2") - if err != nil { - t.Fatalf("unexpected new error %v", err) - } - if "/foo/project/subdir2" != l2.Root() { - t.Fatalf("incorrect root: %s\n", l2.Root()) - } + require.NoError(err) + require.Equal("/foo/project/subdir2", l2.Root()) + x := testCases[2] b, err := l2.Load("fileC.yaml") - if err != nil { - t.Fatalf("unexpected load error %v", err) - } + require.NoError(err) if !reflect.DeepEqual([]byte(x.expectedContent), b) { t.Fatalf("in load expected %s, but got %s", x.expectedContent, b) } @@ -182,21 +154,16 @@ func TestLoaderBadRelative(t *testing.T) { // It's not OK to go over to a previously visited directory. // Must disallow going back and forth in a cycle. l1, err = l2.New("../subdir1") - if err == nil { - t.Fatalf("expected err, but got root %s", l1.Root()) - } + require.Error(err) } func TestLoaderMisc(t *testing.T) { l := makeLoader() _, err := l.New("") - if err == nil { - t.Fatalf("Expected error for empty root location not returned") - } + require.Error(t, err) + _, err = l.New("https://google.com/project") - if err == nil { - t.Fatalf("Expected error") - } + require.Error(t, err) } const ( @@ -239,48 +206,35 @@ func commonSetupForLoaderRestrictionTest(t *testing.T) (string, filesys.FileSyst func doSanityChecksAndDropIntoBase( t *testing.T, l ifc.Loader) ifc.Loader { t.Helper() + require := require.New(t) + data, err := l.Load(path.Join("base", "okayData")) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if string(data) != contentOk { - t.Fatalf("unexpected content: %v", data) - } + require.NoError(err) + require.Equal(contentOk, string(data)) + data, err = l.Load("exteriorData") - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if string(data) != contentExteriorData { - t.Fatalf("unexpected content: %v", data) - } + require.NoError(err) + require.Equal(contentExteriorData, string(data)) // Drop in. l, err = l.New("base") - if err != nil { - t.Fatalf("unexpected error: %v", err) - } + require.NoError(err) // Reading okayData works. data, err = l.Load("okayData") - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if string(data) != contentOk { - t.Fatalf("unexpected content: %v", data) - } + require.NoError(err) + require.Equal(contentOk, string(data)) // Reading local symlink to okayData works. data, err = l.Load("symLinkToOkayData") - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if string(data) != contentOk { - t.Fatalf("unexpected content: %v", data) - } + require.NoError(err) + require.Equal(contentOk, string(data)) + return l } func TestRestrictionRootOnlyInRealLoader(t *testing.T) { + require := require.New(t) dir, fSys := commonSetupForLoaderRestrictionTest(t) var l ifc.Loader @@ -291,22 +245,14 @@ func TestRestrictionRootOnlyInRealLoader(t *testing.T) { // Reading symlink to exteriorData fails. _, err := l.Load("symLinkToExteriorData") - if err == nil { - t.Fatalf("expected error") - } - if !strings.Contains(err.Error(), "is not in or below") { - t.Fatalf("unexpected err: %v", err) - } + require.Error(err) + require.Contains(err.Error(), "is not in or below") // Attempt to read "up" fails, though earlier we were // able to read this file when root was "..". _, err = l.Load("../exteriorData") - if err == nil { - t.Fatalf("expected error") - } - if !strings.Contains(err.Error(), "is not in or below") { - t.Fatalf("unexpected err: %v", err) - } + require.Error(err) + require.Contains(err.Error(), "is not in or below") } func TestRestrictionNoneInRealLoader(t *testing.T) { @@ -320,15 +266,11 @@ func TestRestrictionNoneInRealLoader(t *testing.T) { // Reading symlink to exteriorData works. _, err := l.Load("symLinkToExteriorData") - if err != nil { - t.Fatalf("unexpected error: %v", err) - } + require.NoError(t, err) // Attempt to read "up" works. _, err = l.Load("../exteriorData") - if err != nil { - t.Fatalf("unexpected error: %v", err) - } + require.NoError(t, err) } func splitOnNthSlash(v string, n int) (string, string) { @@ -358,6 +300,8 @@ func TestSplit(t *testing.T) { } func TestNewLoaderAtGitClone(t *testing.T) { + require := require.New(t) + rootURL := "github.com/someOrg/someRepo" pathInRepo := "foo/base" url := rootURL + "/" + pathInRepo @@ -373,40 +317,31 @@ whatever `)) repoSpec, err := git.NewRepoSpecFromURL(url) - if err != nil { - t.Fatalf("unexpected err: %v\n", err) - } + require.NoError(err) + l, err := newLoaderAtGitClone( repoSpec, fSys, nil, git.DoNothingCloner(filesys.ConfirmedDir(coRoot))) - if err != nil { - t.Fatalf("unexpected err: %v\n", err) - } - if coRoot+"/"+pathInRepo != l.Root() { - t.Fatalf("expected root '%s', got '%s'\n", - coRoot+"/"+pathInRepo, l.Root()) - } - if _, err = l.New(url); err == nil { - t.Fatalf("expected cycle error 1") - } - if _, err = l.New(rootURL + "/" + "foo"); err == nil { - t.Fatalf("expected cycle error 2") - } + require.NoError(err) + require.Equal(coRoot+"/"+pathInRepo, l.Root()) + + _, err = l.New(url) + require.Error(err) + + _, err = l.New(rootURL + "/" + "foo") + require.Error(err) pathInRepo = "foo/overlay" fSys.MkdirAll(coRoot + "/" + pathInRepo) url = rootURL + "/" + pathInRepo l2, err := l.New(url) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if coRoot+"/"+pathInRepo != l2.Root() { - t.Fatalf("expected root '%s', got '%s'\n", - coRoot+"/"+pathInRepo, l2.Root()) - } + require.NoError(err) + require.Equal(coRoot+"/"+pathInRepo, l2.Root()) } func TestLoaderDisallowsLocalBaseFromRemoteOverlay(t *testing.T) { + require := require.New(t) + // Define an overlay-base structure in the file system. topDir := "/whatever" cloneRoot := topDir + "/someClone" @@ -421,23 +356,15 @@ func TestLoaderDisallowsLocalBaseFromRemoteOverlay(t *testing.T) { // to the local bases. l1 = newLoaderOrDie( RestrictionRootOnly, fSys, cloneRoot+"/foo/overlay") - if l1.Root() != cloneRoot+"/foo/overlay" { - t.Fatalf("unexpected root %s", l1.Root()) - } + require.Equal(cloneRoot+"/foo/overlay", l1.Root()) + l2, err := l1.New("../base") - if err != nil { - t.Fatalf("unexpected err: %v\n", err) - } - if l2.Root() != cloneRoot+"/foo/base" { - t.Fatalf("unexpected root %s", l2.Root()) - } + require.NoError(nil) + require.Equal(cloneRoot+"/foo/base", l2.Root()) + l3, err := l2.New("../../../highBase") - if err != nil { - t.Fatalf("unexpected err: %v\n", err) - } - if l3.Root() != topDir+"/highBase" { - t.Fatalf("unexpected root %s", l3.Root()) - } + require.NoError(err) + require.Equal(topDir+"/highBase", l3.Root()) // Establish that a Kustomization found in cloned // repo can reach (non-remote) bases inside the clone @@ -452,38 +379,29 @@ func TestLoaderDisallowsLocalBaseFromRemoteOverlay(t *testing.T) { // for local K's. repoSpec, err := git.NewRepoSpecFromURL( "github.com/someOrg/someRepo/foo/overlay") - if err != nil { - t.Fatalf("unexpected err: %v\n", err) - } + require.NoError(err) + l1, err = newLoaderAtGitClone( repoSpec, fSys, nil, git.DoNothingCloner(filesys.ConfirmedDir(cloneRoot))) - if err != nil { - t.Fatalf("unexpected err: %v\n", err) - } - if l1.Root() != cloneRoot+"/foo/overlay" { - t.Fatalf("unexpected root %s", l1.Root()) - } + require.NoError(err) + require.Equal(cloneRoot+"/foo/overlay", l1.Root()) + // This is okay. l2, err = l1.New("../base") - if err != nil { - t.Fatalf("unexpected err: %v\n", err) - } - if l2.Root() != cloneRoot+"/foo/base" { - t.Fatalf("unexpected root %s", l2.Root()) - } + require.NoError(err) + require.Equal(cloneRoot+"/foo/base", l2.Root()) + // This is not okay. _, err = l2.New("../../../highBase") - if err == nil { - t.Fatalf("expected err") - } - if !strings.Contains(err.Error(), - "base '/whatever/highBase' is outside '/whatever/someClone'") { - t.Fatalf("unexpected err: %v", err) - } + require.Error(err) + require.Contains(err.Error(), + "base '/whatever/highBase' is outside '/whatever/someClone'") } func TestLocalLoaderReferencingGitBase(t *testing.T) { + require := require.New(t) + topDir := "/whatever" cloneRoot := topDir + "/someClone" fSys := filesys.MakeFsInMemory() @@ -491,25 +409,21 @@ func TestLocalLoaderReferencingGitBase(t *testing.T) { fSys.MkdirAll(cloneRoot + "/foo/base") root, err := demandDirectoryRoot(fSys, topDir) - if err != nil { - t.Fatalf("unexpected err: %v\n", err) - } + require.NoError(err) + l1 := newLoaderAtConfirmedDir( RestrictionRootOnly, root, fSys, nil, git.DoNothingCloner(filesys.ConfirmedDir(cloneRoot))) - if l1.Root() != topDir { - t.Fatalf("unexpected root %s", l1.Root()) - } + require.Equal(topDir, l1.Root()) + l2, err := l1.New("github.com/someOrg/someRepo/foo/base") - if err != nil { - t.Fatalf("unexpected err: %v\n", err) - } - if l2.Root() != cloneRoot+"/foo/base" { - t.Fatalf("unexpected root %s", l2.Root()) - } + require.NoError(err) + require.Equal(cloneRoot+"/foo/base", l2.Root()) } func TestRepoDirectCycleDetection(t *testing.T) { + require := require.New(t) + topDir := "/cycles" cloneRoot := topDir + "/someClone" fSys := filesys.MakeFsInMemory() @@ -517,28 +431,24 @@ func TestRepoDirectCycleDetection(t *testing.T) { fSys.MkdirAll(cloneRoot) root, err := demandDirectoryRoot(fSys, topDir) - if err != nil { - t.Fatalf("unexpected err: %v\n", err) - } + require.NoError(err) + l1 := newLoaderAtConfirmedDir( RestrictionRootOnly, root, fSys, nil, git.DoNothingCloner(filesys.ConfirmedDir(cloneRoot))) p1 := "github.com/someOrg/someRepo/foo" rs1, err := git.NewRepoSpecFromURL(p1) - if err != nil { - t.Fatalf("unexpected err: %v", err) - } + require.NoError(err) + l1.repoSpec = rs1 _, err = l1.New(p1) - if err == nil { - t.Fatalf("expected error") - } - if !strings.Contains(err.Error(), "cycle detected") { - t.Fatalf("unexpected err: %v", err) - } + require.Error(err) + require.Contains(err.Error(), "cycle detected") } func TestRepoIndirectCycleDetection(t *testing.T) { + require := require.New(t) + topDir := "/cycles" cloneRoot := topDir + "/someClone" fSys := filesys.MakeFsInMemory() @@ -546,9 +456,8 @@ func TestRepoIndirectCycleDetection(t *testing.T) { fSys.MkdirAll(cloneRoot) root, err := demandDirectoryRoot(fSys, topDir) - if err != nil { - t.Fatalf("unexpected err: %v", err) - } + require.NoError(err) + l0 := newLoaderAtConfirmedDir( RestrictionRootOnly, root, fSys, nil, git.DoNothingCloner(filesys.ConfirmedDir(cloneRoot))) @@ -557,20 +466,14 @@ func TestRepoIndirectCycleDetection(t *testing.T) { p2 := "github.com/someOrg/someRepo2" l1, err := l0.New(p1) - if err != nil { - t.Fatalf("unexpected err: %v", err) - } + require.NoError(err) + l2, err := l1.New(p2) - if err != nil { - t.Fatalf("unexpected err: %v", err) - } + require.NoError(err) + _, err = l2.New(p1) - if err == nil { - t.Fatalf("expected error") - } - if !strings.Contains(err.Error(), "cycle detected") { - t.Fatalf("unexpected err: %v", err) - } + require.Error(err) + require.Contains(err.Error(), "cycle detected") } // Inspired by https://hassansin.github.io/Unit-Testing-http-client-in-Go @@ -588,6 +491,8 @@ func makeFakeHTTPClient(fn fakeRoundTripper) *http.Client { // TestLoaderHTTP test http file loader func TestLoaderHTTP(t *testing.T) { + require := require.New(t) + var testCasesFile = []testData{ { path: "http/file.yaml", @@ -596,14 +501,11 @@ func TestLoaderHTTP(t *testing.T) { } l1 := NewFileLoaderAtRoot(MakeFakeFs(testCasesFile)) - if "/" != l1.Root() { - t.Fatalf("incorrect root: '%s'\n", l1.Root()) - } + require.Equal("/", l1.Root()) + for _, x := range testCasesFile { b, err := l1.Load(x.path) - if err != nil { - t.Fatalf("unexpected load error: %v", err) - } + require.NoError(err) if !reflect.DeepEqual([]byte(x.expectedContent), b) { t.Fatalf("in load expected %s, but got %s", x.expectedContent, b) } @@ -623,9 +525,7 @@ func TestLoaderHTTP(t *testing.T) { for _, x := range testCasesHTTP { hc := makeFakeHTTPClient(func(req *http.Request) *http.Response { u := req.URL.String() - if x.path != u { - t.Fatalf("expected URL %s, but got %s", x.path, u) - } + require.Equal(x.path, u) return &http.Response{ StatusCode: 200, Body: ioutil.NopCloser(bytes.NewBufferString(x.expectedContent)), @@ -635,9 +535,7 @@ func TestLoaderHTTP(t *testing.T) { l2 := l1 l2.http = hc b, err := l2.Load(x.path) - if err != nil { - t.Fatalf("unexpected load error: %v", err) - } + require.NoError(err) if !reflect.DeepEqual([]byte(x.expectedContent), b) { t.Fatalf("in load expected %s, but got %s", x.expectedContent, b) } @@ -657,8 +555,6 @@ func TestLoaderHTTP(t *testing.T) { l2 := l1 l2.http = hc _, err := l2.Load(x.path) - if err == nil { - t.Fatalf("expect error but get %v", err) - } + require.Error(err) } }