From 8f6ac3a73a090d3833d1013c2de37e980001ba3c Mon Sep 17 00:00:00 2001 From: Benjamin Isinger Date: Fri, 25 Aug 2023 17:50:38 +0200 Subject: [PATCH] Fix check run in case no version was provided Fail early if no run was provided Refactor check test code --- internal/dtr/check.go | 9 +- internal/dtr/check_test.go | 182 +++++++++++++++++++++++-------------- internal/dtr/common.go | 8 +- 3 files changed, 124 insertions(+), 75 deletions(-) diff --git a/internal/dtr/check.go b/internal/dtr/check.go index 2c22398..ec6501e 100644 --- a/internal/dtr/check.go +++ b/internal/dtr/check.go @@ -41,8 +41,13 @@ func Check(in io.Reader) (CheckResult, error) { } if len(result) == 0 { - // Only return the incoming version to signal concourse that there is nothing new - result = append(result, config.Version) + if config.Version != nil { + // Only return the incoming version to signal concourse that there is nothing new + result = append(result, config.Version) + } else { + // Return empty result as we don't have anything + result = CheckResult{} + } } return result, nil diff --git a/internal/dtr/check_test.go b/internal/dtr/check_test.go index 5ff9b80..401b5de 100644 --- a/internal/dtr/check_test.go +++ b/internal/dtr/check_test.go @@ -36,89 +36,133 @@ var _ = Describe("Check", func() { }) Context("valid configuration", func() { - It("should just fail nicely if the provided command returns with non-zero exit code", func() { - _, err := Check(feed(Config{ - Source: Source{ - Check: Custom{Run: "false"}, - }, - })) + var ( + result CheckResult + err error + config Config + ) + + JustBeforeEach(func() { + result, err = Check(feed(config)) + }) - Expect(err).To(HaveOccurred()) + When("command returns with non-zero exit code", func() { + BeforeEach(func() { + config.Source.Check.Run = "false" + }) + + It("should just fail nicely", func() { + Expect(err).To(HaveOccurred()) + }) }) - It("should return provided version when no line is return by run", func() { - versionIn := Version{"ref": "barfoo"} - result, err := Check(feed(Config{ - Source: Source{ - Check: Custom{Run: "true"}, - }, - Version: versionIn, - })) - - Expect(err).ToNot(HaveOccurred()) - Expect(result).To(Equal(CheckResult{versionIn})) + When("no line is returned by run", func() { + BeforeEach(func() { + config = Config{ + Source: Source{ + Check: Custom{Run: "true"}, + }, + } + }) + + When("version is provided by input", func() { + var versionIn Version + + BeforeEach(func() { + versionIn = Version{"ref": "barfoo"} + config.Version = versionIn + }) + + It("should return provided version", func() { + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(Equal(CheckResult{versionIn})) + }) + }) + + When("no version is provided by input", func() { + BeforeEach(func() { + config.Version = nil + }) + + It("should return empty result", func() { + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(Equal(CheckResult{})) + }) + }) }) - It("should return two versions when two lines are returned", func() { - result, err := Check(feed(Config{ - Source: Source{ - Check: Custom{Run: "echo foobar && echo barfoo"}, - }, - })) - - Expect(err).ToNot(HaveOccurred()) - Expect(result).To(Equal(CheckResult{ - Version{"ref": "foobar"}, - Version{"ref": "barfoo"}, - })) + When("command returns two lines", func() { + BeforeEach(func() { + config.Source.Check.Run = "echo foobar && echo barfoo" + }) + + It("should return two versions", func() { + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(Equal(CheckResult{ + Version{"ref": "foobar"}, + Version{"ref": "barfoo"}, + })) + }) }) - It("should run before commands before returning a version", func() { - result, err := Check(feed(Config{ - Source: Source{ - Check: Custom{ - Before: "echo foobar >/tmp/version", - Run: "cat /tmp/version", - }, - }, - })) + When("run command is not defined", func() { + BeforeEach(func() { + config.Source.Check = Custom{ + Run: "", + } + }) - Expect(err).ToNot(HaveOccurred()) - Expect(result).To(Equal(CheckResult{ - Version{"ref": "foobar"}, - })) + It("should just fail nicely", func() { + Expect(err).To(HaveOccurred()) + }) }) - It("should just fail nicely if the provided before command return with non-zero exit code", func() { - _, err := Check(feed(Config{ - Source: Source{ - Check: Custom{ - Before: "false", - Run: "true", - }, - }, - })) + When("before command is defined", func() { + BeforeEach(func() { + config.Source.Check = Custom{ + Before: "echo foobar >/tmp/version", + Run: "cat /tmp/version", + } + }) + + It("should run before commands before returning a version", func() { + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(Equal(CheckResult{ + Version{"ref": "foobar"}, + })) + }) + }) - Expect(err).To(HaveOccurred()) + When("before command returns with non-zero exit code", func() { + BeforeEach(func() { + config.Source.Check = Custom{ + Before: "false", + Run: "true", + } + }) + + It("should just fail nicely", func() { + Expect(err).To(HaveOccurred()) + }) }) - It("should set environment variables that can be used in run", func() { - result, err := Check(feed(Config{ - Source: Source{ - Check: Custom{ - Env: map[string]string{ - "FOO": "foo", - "BAR": "bar", - }, - Run: "echo ${FOO}${BAR}", + When("ENV is configured", func() { + BeforeEach(func() { + config.Source.Check = Custom{ + Env: map[string]string{ + "FOO": "foo", + "BAR": "bar", }, - }, - })) - - Expect(err).ToNot(HaveOccurred()) - Expect(result).To(Equal(CheckResult{ - Version{"ref": "foobar"}, - })) + Run: "echo ${FOO}${BAR}", + } + }) + + It("should set environment variables that can be used in run", func() { + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(Equal(CheckResult{ + Version{"ref": "foobar"}, + })) + }) }) }) }) diff --git a/internal/dtr/common.go b/internal/dtr/common.go index fbbba29..1f0670b 100644 --- a/internal/dtr/common.go +++ b/internal/dtr/common.go @@ -104,6 +104,10 @@ func execute(entry Custom) ([]string, error) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() + if entry.Run == "" { + return nil, fmt.Errorf("run command not specified. Bailing out") + } + if entry.Before != "" { before := command(ctx, entry.Env, entry.Before, os.Stderr) if err := before.Run(); err != nil { @@ -111,10 +115,6 @@ func execute(entry Custom) ([]string, error) { } } - if entry.Run == "" { - return nil, fmt.Errorf("run command not specified. Bailing out") - } - var outStream bytes.Buffer run := command(ctx, entry.Env, entry.Run, &outStream) if err := run.Run(); err != nil {