From 23222217c8c78d64c1737cc573d10b66d143ad55 Mon Sep 17 00:00:00 2001 From: Chrstopher Hunter <8398225+crhntr@users.noreply.github.com> Date: Wed, 24 Jul 2024 14:43:22 -0700 Subject: [PATCH 1/4] test(bake): add coverage for missing --output scenario --- internal/commands/bake_test.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/internal/commands/bake_test.go b/internal/commands/bake_test.go index 45e1b3b7..32036a98 100644 --- a/internal/commands/bake_test.go +++ b/internal/commands/bake_test.go @@ -398,6 +398,18 @@ var _ = Describe("Bake", func() { Expect(string(fakeBakeRecordFunc.productTemplate)).To(Equal("some-interpolated-metadata"), "it gives the bake recorder the product template") }) + FContext("when the output flag is not set", func() { + When("the tile-name flag is not provided", func() { + It("uses the tile as the filename prefix", func() { + err := bake.Execute([]string{}) + Expect(err).To(Not(HaveOccurred())) + Expect(fakeTileWriter.WriteCallCount()).To(Equal(1)) + _, input := fakeTileWriter.WriteArgsForCall(0) + Expect(input.OutputFile).To(Equal(filepath.Join("tile-v1.2.3.pivotal"))) + }) + }) + }) + Context("when bake configuration is in the Kilnfile", func() { BeforeEach(func() { bake = bake.WithKilnfileFunc(func(s string) (cargo.Kilnfile, error) { @@ -448,6 +460,7 @@ var _ = Describe("Bake", func() { When("a the tile flag is passed", func() { It("it uses the value from the bake configuration with the correct name", func() { err := bake.Execute([]string{ + "bake", "--tile-name=p-each", }) Expect(err).To(Not(HaveOccurred())) From 6a39589212f9315c55bbfa42662006738b70a902 Mon Sep 17 00:00:00 2001 From: Chrstopher Hunter <8398225+crhntr@users.noreply.github.com> Date: Wed, 24 Jul 2024 15:13:46 -0700 Subject: [PATCH 2/4] feat(bake): generate better file names fixes: #437 --- internal/commands/bake.go | 22 ++++++------- internal/commands/bake_test.go | 59 +++++++++++++++++++++++++--------- 2 files changed, 53 insertions(+), 28 deletions(-) diff --git a/internal/commands/bake.go b/internal/commands/bake.go index 00de823b..71ffb5d6 100644 --- a/internal/commands/bake.go +++ b/internal/commands/bake.go @@ -348,27 +348,23 @@ func (b *Bake) loadFlags(args []string) error { } if shouldReadVersionFile(b, args) { - fileInfo, err := b.fs.Stat("version") - // TODO: test this - if fileInfo != nil && err == nil { - var file File - file, err = b.fs.Open(fileInfo.Name()) - if err != nil && file == nil { - return err - } + file, err := b.fs.Open("version") + if err == nil { defer closeAndIgnoreError(file) - - versionBuf := make([]byte, fileInfo.Size()) - _, _ = file.Read(versionBuf) + versionBuf, _ := io.ReadAll(file) b.Options.Version = strings.TrimSpace(string(versionBuf)) } } if shouldGenerateTileFileName(b, args) { - b.Options.OutputFile = "tile.pivotal" + prefix := "tile" + if b.Options.TileName != "" { + prefix = b.Options.TileName + } if b.Options.Version != "" { - b.Options.OutputFile = "tile-" + b.Options.Version + ".pivotal" + prefix += "-" + b.Options.Version } + b.Options.OutputFile = prefix + ".pivotal" } if shouldNotUseDefaultKilnfileFlag(args) { diff --git a/internal/commands/bake_test.go b/internal/commands/bake_test.go index 32036a98..8619d91f 100644 --- a/internal/commands/bake_test.go +++ b/internal/commands/bake_test.go @@ -10,6 +10,9 @@ import ( "testing" "time" + "github.com/go-git/go-billy/v5" + "github.com/go-git/go-billy/v5/memfs" + "github.com/pivotal-cf/kiln/pkg/cargo" "github.com/pivotal-cf/kiln/pkg/bake" @@ -51,7 +54,8 @@ var _ = Describe("Bake", func() { fakeTileWriter *fakes.TileWriter fakeChecksummer *fakes.Checksummer - fakeFilesystem *fakes.FileSystem + fakeFilesystem billy.Filesystem + fakeHomeDirFunc func() (string, error) otherReleasesDirectory string @@ -97,15 +101,14 @@ var _ = Describe("Bake", func() { fakeJobsService = &fakes.MetadataTemplatesParser{} fakePropertiesService = &fakes.MetadataTemplatesParser{} fakeRuntimeConfigsService = &fakes.MetadataTemplatesParser{} - fakeFilesystem = &fakes.FileSystem{} - fakeVersionInfo := &fakes.FileInfo{} - fileVersion := "some-version" - fakeVersionInfo.SizeReturns(int64(len(fileVersion))) - fakeVersionInfo.NameReturns("version") - fakeFilesystem.StatReturns(fakeVersionInfo, nil) - result1 := &fakes.File{} - result1.ReadReturns(0, nil) - fakeFilesystem.OpenReturns(result1, nil) + + { + fakeFilesystem = memfs.New() + file, _ := fakeFilesystem.Create("version") + _, _ = file.Write([]byte("1.2.3")) + _ = file.Close() + } + fakeHomeDirFunc = func() (string, error) { return "/home/", nil } @@ -398,14 +401,41 @@ var _ = Describe("Bake", func() { Expect(string(fakeBakeRecordFunc.productTemplate)).To(Equal("some-interpolated-metadata"), "it gives the bake recorder the product template") }) - FContext("when the output flag is not set", func() { - When("the tile-name flag is not provided", func() { + Context("when the output flag is not set", func() { + When("no version is provided", func() { It("uses the tile as the filename prefix", func() { - err := bake.Execute([]string{}) + _ = fakeFilesystem.Remove("version") + err := bake.Execute([]string{ + "--metadata", "some-metadata", + }) + Expect(err).To(Not(HaveOccurred())) + Expect(fakeTileWriter.WriteCallCount()).To(Equal(1)) + _, input := fakeTileWriter.WriteArgsForCall(0) + Expect(input.OutputFile).To(Equal(filepath.Join("tile.pivotal"))) + }) + }) + When("the version flag is provided", func() { + It("uses the tile as the filename prefix", func() { + err := bake.Execute([]string{ + "--metadata", "some-metadata", + "--version", "some-version", + }) + Expect(err).To(Not(HaveOccurred())) + Expect(fakeTileWriter.WriteCallCount()).To(Equal(1)) + _, input := fakeTileWriter.WriteArgsForCall(0) + Expect(input.OutputFile).To(Equal(filepath.Join("tile-some-version.pivotal"))) + }) + }) + When("the tile name is provided", func() { + It("uses the tile as the filename prefix", func() { + err := bake.Execute([]string{ + "--metadata", "some-metadata", + "--tile-name", "p-rune", + }) Expect(err).To(Not(HaveOccurred())) Expect(fakeTileWriter.WriteCallCount()).To(Equal(1)) _, input := fakeTileWriter.WriteArgsForCall(0) - Expect(input.OutputFile).To(Equal(filepath.Join("tile-v1.2.3.pivotal"))) + Expect(input.OutputFile).To(Equal(filepath.Join("p-rune-1.2.3.pivotal"))) }) }) }) @@ -460,7 +490,6 @@ var _ = Describe("Bake", func() { When("a the tile flag is passed", func() { It("it uses the value from the bake configuration with the correct name", func() { err := bake.Execute([]string{ - "bake", "--tile-name=p-each", }) Expect(err).To(Not(HaveOccurred())) From faf47d0af134f7078b9ac0dc5ce8f5725baa335e Mon Sep 17 00:00:00 2001 From: Chrstopher Hunter <8398225+crhntr@users.noreply.github.com> Date: Wed, 24 Jul 2024 16:46:50 -0700 Subject: [PATCH 3/4] fix(bake): command tests failed after switcihg to fs based test --- internal/commands/bake_test.go | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/internal/commands/bake_test.go b/internal/commands/bake_test.go index 8619d91f..b4bbff1b 100644 --- a/internal/commands/bake_test.go +++ b/internal/commands/bake_test.go @@ -104,9 +104,15 @@ var _ = Describe("Bake", func() { { fakeFilesystem = memfs.New() - file, _ := fakeFilesystem.Create("version") - _, _ = file.Write([]byte("1.2.3")) - _ = file.Close() + + for name, contents := range map[string]string{ + "version": "1.2.3", + "/home/.kiln/credentials.yml": "", + } { + file, _ := fakeFilesystem.Create(name) + _, _ = file.Write([]byte(contents)) + _ = file.Close() + } } fakeHomeDirFunc = func() (string, error) { @@ -229,9 +235,7 @@ var _ = Describe("Bake", func() { Expect(fakeTemplateVariablesService.FromPathsAndPairsCallCount()).To(Equal(1)) varFiles, variables := fakeTemplateVariablesService.FromPathsAndPairsArgsForCall(0) - Expect(len(varFiles)).To(Equal(2)) - Expect(varFiles[0]).To(Equal("some-variables-file")) - Expect(varFiles[1]).To(Equal("/home/.kiln/credentials.yml")) + Expect(varFiles).To(ConsistOf("some-variables-file", "/home/.kiln/credentials.yml")) Expect(variables).To(Equal([]string{"some-variable=some-variable-value"})) Expect(fakeBOSHVariablesService.ParseMetadataTemplatesCallCount()).To(Equal(1)) @@ -442,6 +446,8 @@ var _ = Describe("Bake", func() { Context("when bake configuration is in the Kilnfile", func() { BeforeEach(func() { + kf, _ := fakeFilesystem.Create("Kilnfile") + _ = kf.Close() bake = bake.WithKilnfileFunc(func(s string) (cargo.Kilnfile, error) { return cargo.Kilnfile{ BakeConfigurations: []cargo.BakeConfiguration{ @@ -468,6 +474,8 @@ var _ = Describe("Bake", func() { }) Context("when bake configuration has multiple options", func() { BeforeEach(func() { + kf, _ := fakeFilesystem.Create("Kilnfile") + _ = kf.Close() bake = bake.WithKilnfileFunc(func(s string) (cargo.Kilnfile, error) { return cargo.Kilnfile{ BakeConfigurations: []cargo.BakeConfiguration{ @@ -628,6 +636,10 @@ var _ = Describe("Bake", func() { }) Context("when Kilnfile is specified", func() { + BeforeEach(func() { + kf, _ := fakeFilesystem.Create("Kilnfile") + _ = kf.Close() + }) It("renders the stemcell criteria in tile metadata from that specified the Kilnfile.lock", func() { outputFile := "some-output-dir/some-product-file-1.2.3-build.4" err := bake.Execute([]string{ @@ -914,6 +926,10 @@ var _ = Describe("Bake", func() { }) Context("when both the --kilnfile and --stemcells-directory are provided", func() { + BeforeEach(func() { + kf, _ := fakeFilesystem.Create("Kilnfile") + _ = kf.Close() + }) It("returns an error", func() { err := bake.Execute([]string{ "--metadata", "some-metadata", @@ -927,6 +943,10 @@ var _ = Describe("Bake", func() { // todo: When --stemcell-tarball is removed, delete this test Context("when both the --stemcell-tarball and --kilnfile are provided", func() { + BeforeEach(func() { + kf, _ := fakeFilesystem.Create("Kilnfile") + _ = kf.Close() + }) It("returns an error", func() { err := bake.Execute([]string{ "--metadata", "some-metadata", From 44a6bad86eb39b549e35964b4ff646dbce0358fe Mon Sep 17 00:00:00 2001 From: Chrstopher Hunter <8398225+crhntr@users.noreply.github.com> Date: Wed, 24 Jul 2024 16:54:58 -0700 Subject: [PATCH 4/4] fix(acceptance): match any tile file name --- .../workflows/scenario/step_funcs_tile.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/internal/acceptance/workflows/scenario/step_funcs_tile.go b/internal/acceptance/workflows/scenario/step_funcs_tile.go index 2e758d67..c642e4e1 100644 --- a/internal/acceptance/workflows/scenario/step_funcs_tile.go +++ b/internal/acceptance/workflows/scenario/step_funcs_tile.go @@ -8,7 +8,7 @@ import ( "fmt" "io/fs" "log" - "os" + "path/filepath" "strings" "github.com/pivotal-cf/kiln/pkg/cargo" @@ -23,12 +23,18 @@ import ( // aTileIsCreated asserts the output tile exists func aTileIsCreated(ctx context.Context) error { - tilePath, err := defaultFilePathForTile(ctx) + p, err := tileRepoPath(ctx) + if err != nil { + return err + } + matches, err := filepath.Glob(filepath.Join(p, "*.pivotal")) if err != nil { return err } - _, err = os.Stat(tilePath) - return err + if len(matches) == 0 { + return errors.New("no tile found") + } + return nil } // theTileContains checks that the filePaths exist in the tile