From dce239738c8ac78b93b515182d4ce366448dcccc Mon Sep 17 00:00:00 2001 From: Nick Rohn Date: Tue, 13 Jun 2023 14:41:42 -0700 Subject: [PATCH 1/3] add stability tests - stability tests differentiate between ert and srt - refactor flags and names to make sense in a world with three units - add env var flags for passthrough to test suites - clean up test containers automatically Co-authored-by: Nick Rohn Co-authored-by: Gary Liu --- internal/commands/test_tile.go | 95 +++++++++---- .../commands/test_tile_acceptance_test.go | 2 +- internal/commands/test_tile_test.go | 134 ++++++++++++++++-- .../tas/test/stability/stability_test.go | 58 ++++++++ 4 files changed, 255 insertions(+), 34 deletions(-) create mode 100644 internal/commands/testdata/tas_fake/tas/test/stability/stability_test.go diff --git a/internal/commands/test_tile.go b/internal/commands/test_tile.go index b7f4e25d3..1158a37c1 100644 --- a/internal/commands/test_tile.go +++ b/internal/commands/test_tile.go @@ -74,11 +74,13 @@ func (l infoLog) Writer() io.Writer { type TileTest struct { Options struct { - TilePath string `short:"tp" long:"tile-path" default:"." description:"Path to the Tile directory (e.g., ~/workspace/tas/ist)."` - GingkoManifestFlags string `short:"gmf" long:"ginkgo-manifest-flags" default:"-r -p -slowSpecThreshold 15" description:"Flags to pass to the Ginkgo Manifest test suite."` - Verbose bool `short:"v" long:"verbose" default:"false" description:"Print info lines. This doesn't affect Ginkgo output."` - ManifestOnly bool ` long:"manifest-only" default:"false" description:"Run only Manifest tests."` - MigrationsOnly bool ` long:"migrations-only" default:"false" description:"Run only Migration tests."` + TilePath string `short:"tp" long:"tile-path" default:"." description:"Path to the Tile directory (e.g., ~/workspace/tas/ist)."` + GingkoFlags string `short:"gmf" long:"ginkgo-flags" default:"-r -p -slowSpecThreshold 15" description:"Flags to pass to the Ginkgo Manifest and Stability test suites."` + Verbose bool `short:"v" long:"verbose" default:"false" description:"Print info lines. This doesn't affect Ginkgo output."` + Manifest bool ` long:"manifest" default:"false" description:"Focus the Manifest tests."` + Migrations bool ` long:"migrations" default:"false" description:"Focus the Migration tests."` + Stability bool ` long:"stability" default:"false" description:"Focus the Stability tests."` + EnvironmentVars []string `short:"e" long:"environment-variable" description:"Pass environment variable to the test suites. For example --stability -e 'PRODUCT=srt'."` } logger *log.Logger @@ -110,6 +112,10 @@ func (u TileTest) Execute(args []string) error { if err != nil { return fmt.Errorf("could not parse manifest-test flags: %s", err) } + envMap, err := validateAndParseEnvVars(u.Options.EnvironmentVars) + if err != nil { + return fmt.Errorf("could not parse manifest-test flags: %s", err) + } loggerWithInfo := infoLog{ logger: u.logger, @@ -174,8 +180,7 @@ func (u TileTest) Execute(args []string) error { if buildError != "" { if strings.Contains(buildError, "exit code: 128") { format := `Does your private key have access to the ops-manifest repo?\n error: %s - Automatically looking in %s for ssh keys. We use SSH_AUTH_SOCK environment variable - for the socket.` + Automatically looking in %s for ssh keys. SSH_AUTH_SOCK needs to be set.` return fmt.Errorf(format, buildError, strings.Join(StandardSSHKeys, ", ")) } return errors.New(buildError) @@ -192,27 +197,34 @@ func (u TileTest) Execute(args []string) error { loggerWithInfo.Info("Mounting", parentDir, "and testing", tileDir) - runAll := !u.Options.ManifestOnly && !u.Options.MigrationsOnly + runAll := !u.Options.Manifest && !u.Options.Migrations && !u.Options.Stability var dockerCmds []string - if u.Options.ManifestOnly || runAll { - dockerCmds = append(dockerCmds, fmt.Sprintf("cd /tas/%s/test/manifest", tileDir)) - dockerCmds = append(dockerCmds, fmt.Sprintf("PRODUCT=%s RENDERER=ops-manifest ginkgo %s", toProduct(tileDir), u.Options.GingkoManifestFlags)) - } - if u.Options.MigrationsOnly || runAll { + if u.Options.Migrations || runAll { dockerCmds = append(dockerCmds, fmt.Sprintf("cd /tas/%s/migrations", tileDir)) dockerCmds = append(dockerCmds, "npm install") dockerCmds = append(dockerCmds, "npm test") } + ginkgo := []string{} + if u.Options.Stability || runAll { + ginkgo = append(ginkgo, fmt.Sprintf("/tas/%s/test/stability", tileDir)) + } + if u.Options.Manifest || runAll { + ginkgo = append(ginkgo, fmt.Sprintf("/tas/%s/test/manifest", tileDir)) + } + if u.Options.Stability || u.Options.Manifest || runAll { + ginkgoCommand := fmt.Sprintf("cd /tas/%s && ginkgo %s %s", tileDir, u.Options.GingkoFlags, strings.Join(ginkgo, " ")) + dockerCmds = append(dockerCmds, ginkgoCommand) + } dockerCmd := strings.Join(dockerCmds, " && ") - - envVars := getTileTestEnvVars(absRepoDir, tileDir) loggerWithInfo.Info("Running:", dockerCmd) + envVars := getTileTestEnvVars(absRepoDir, tileDir, envMap) + fmt.Printf("%+v\n", envVarsToSlice(envVars)) createResp, err := u.mobi.ContainerCreate(u.ctx, &container.Config{ Image: "kiln_test_dependencies:vmware", Cmd: []string{"/bin/bash", "-c", dockerCmd}, - Env: envVars, + Env: envVarsToSlice(envVars), Tty: true, }, &container.HostConfig{ LogConfig: container.LogConfig{ @@ -276,6 +288,18 @@ func (u TileTest) Execute(args []string) error { return nil } +func validateAndParseEnvVars(environmentVarArgs []string) (environmentVars, error) { + envMap := make(environmentVars) + for _, envVar := range environmentVarArgs { + parts := strings.SplitN(envVar, "=", 2) + if len(parts) != 2 { + return nil, errors.New("Environment variables must have the format [key]=[value]") + } + envMap[parts[0]] = parts[1] + } + return envMap, nil +} + func toProduct(dir string) string { switch dir { case "tas": @@ -287,21 +311,32 @@ func toProduct(dir string) string { } } -func getTileTestEnvVars(dir, productDir string) []string { +func getTileTestEnvVars(dir, productDir string, envMap environmentVars) environmentVars { const fixturesFormat = "%s/test/manifest/fixtures" metadataPath := fmt.Sprintf(fixturesFormat+"/tas_metadata.yml", dir) configPath := fmt.Sprintf(fixturesFormat+"/tas_config.yml", dir) - _, configErr := os.Stat(configPath) _, metadataErr := os.Stat(metadataPath) + + envVarsMap := make(map[string]string) if metadataErr == nil && configErr == nil { - return []string{ - fmt.Sprintf("TAS_METADATA_PATH=%s", fmt.Sprintf(fixturesFormat+"/%s", "/tas/"+productDir, "tas_metadata.yml")), - fmt.Sprintf("TAS_CONFIG_FILE=%s", fmt.Sprintf(fixturesFormat+"/%s", "/tas/"+productDir, "tas_config.yml")), - } + envVarsMap["TAS_METADATA_PATH"] = fmt.Sprintf(fixturesFormat+"/%s", "/tas/"+productDir, "tas_metadata.yml") + envVarsMap["TAS_CONFIG_FILE"] = fmt.Sprintf(fixturesFormat+"/%s", "/tas/"+productDir, "tas_config.yml") } - return nil + // no need to set for tas tile, since it defaults to ert. + // for ist and tasw, we need to set it, as there's no default. + if toProduct(productDir) != "ert" { + envVarsMap["PRODUCT"] = toProduct(productDir) + } + envVarsMap["RENDERER"] = "ops-manifest" + + // overwrite with / include optional env vars + for k, v := range envMap { + envVarsMap[k] = v + } + + return envVarsMap } func getTarReader(fileContents string) (*bufio.Reader, error) { @@ -365,14 +400,24 @@ func (u TileTest) addMissingKeys() error { return err } +type environmentVars map[string]string + +func envVarsToSlice(envVars environmentVars) []string { + convertedEnvVars := []string{} + for k, v := range envVars { + convertedEnvVars = append(convertedEnvVars, fmt.Sprintf("%s=%s", k, v)) + } + return convertedEnvVars +} + type ErrorLine struct { Error string `json:"error"` } func (u TileTest) Usage() jhanda.Usage { return jhanda.Usage{ - Description: "Tests the Manifest and Migrations for a Tile in a Docker container. Requires a Docker daemon to be running and ssh keys with access to Ops Manager's Git repository. For non-interactive use, either set the environment variable SSH_PASSWORD, or `ssh add` your identity before running.", - ShortDescription: "Tests the Manifest and Migrations for a Tile.", + Description: "Run the Manifest, Migrations, and Stability tests for a Tile in a Docker container. Requires a Docker daemon to be running and ssh keys with access to Ops Manager's Git repository. For non-interactive use, either set the environment variable SSH_PASSWORD, or `ssh add` your identity before running.", + ShortDescription: "Runs unit tests for a Tile.", Flags: u.Options, } } diff --git a/internal/commands/test_tile_acceptance_test.go b/internal/commands/test_tile_acceptance_test.go index 0a9dd7bbc..24313a6d3 100644 --- a/internal/commands/test_tile_acceptance_test.go +++ b/internal/commands/test_tile_acceptance_test.go @@ -55,7 +55,7 @@ var _ = Describe("test", func() { Expect(err).To(HaveOccurred()) Expect(testOutput.String()).NotTo(ContainSubstring("SUCCESS")) - Expect(testOutput.String()).To(ContainSubstring("Failure")) + Expect(testOutput.String()).To(Or(ContainSubstring("ERR!"), ContainSubstring("Failure"))) }) }) }) diff --git a/internal/commands/test_tile_test.go b/internal/commands/test_tile_test.go index a05140495..73aa5c3b7 100644 --- a/internal/commands/test_tile_test.go +++ b/internal/commands/test_tile_test.go @@ -8,6 +8,7 @@ import ( "log" "os" "path/filepath" + "sort" "strings" "github.com/docker/docker/api/types" @@ -75,7 +76,7 @@ var _ = Describe("kiln test docker", func() { }) When("verbose is passed", func() { It("succeeds and logs info", func() { - err := subjectUnderTest.Execute([]string{"--manifest-only", "--verbose", "--tile-path", helloTilePath, "--ginkgo-manifest-flags", "-r -slowSpecThreshold 1"}) + err := subjectUnderTest.Execute([]string{"--manifest", "--verbose", "--tile-path", helloTilePath, "--ginkgo-flags", "-r -slowSpecThreshold 1"}) Expect(err).To(BeNil()) By("logging helpful messages", func() { @@ -92,13 +93,30 @@ var _ = Describe("kiln test docker", func() { Expect(fakeMobyClient.ContainerCreateCallCount()).To(Equal(1)) _, config, _, _, _, _ := fakeMobyClient.ContainerCreateArgsForCall(0) By("configuring the metadata and product config when they exist", func() { - Expect(config.Env).To(Equal([]string{ + expected := []string{ + "PRODUCT=hello-tile", + "RENDERER=ops-manifest", "TAS_METADATA_PATH=/tas/hello-tile/test/manifest/fixtures/tas_metadata.yml", "TAS_CONFIG_FILE=/tas/hello-tile/test/manifest/fixtures/tas_config.yml", - })) + } + actual := config.Env + sort.Strings(actual) + sort.Strings(expected) + + Expect(actual).To(BeEquivalentTo(expected)) }) By("executing the tests", func() { - dockerCmd := fmt.Sprintf("cd /tas/%s/test/manifest && PRODUCT=%[1]s RENDERER=ops-manifest ginkgo -r -slowSpecThreshold 1", "hello-tile") + expected := []string{ + "PRODUCT=hello-tile", + "RENDERER=ops-manifest", + "TAS_METADATA_PATH=/tas/hello-tile/test/manifest/fixtures/tas_metadata.yml", + "TAS_CONFIG_FILE=/tas/hello-tile/test/manifest/fixtures/tas_config.yml", + } + actual := config.Env + sort.Strings(actual) + sort.Strings(expected) + + dockerCmd := "cd /tas/hello-tile && ginkgo -r -slowSpecThreshold 1 /tas/hello-tile/test/manifest" Expect(config.Cmd).To(Equal(strslice.StrSlice{"/bin/bash", "-c", dockerCmd})) }) }) @@ -106,7 +124,7 @@ var _ = Describe("kiln test docker", func() { }) When("verbose isn't passed", func() { It("doesn't log info", func() { - err := subjectUnderTest.Execute([]string{"--manifest-only", "--tile-path", helloTilePath, "--ginkgo-manifest-flags", "-r -slowSpecThreshold 1"}) + err := subjectUnderTest.Execute([]string{"--manifest", "--tile-path", helloTilePath, "--ginkgo-flags", "-r -slowSpecThreshold 1"}) Expect(err).To(BeNil()) By("logging helpful messages", func() { @@ -134,7 +152,7 @@ var _ = Describe("kiln test docker", func() { }) It("returns an error", func() { subjectUnderTest := commands.NewTileTest(logger, ctx, fakeMobyClient, fakeSshProvider) - err := subjectUnderTest.Execute([]string{"--manifest-only", "--verbose", "--tile-path", helloTilePath, "--ginkgo-manifest-flags", "-r -slowSpecThreshold 1"}) + err := subjectUnderTest.Execute([]string{"--manifest", "--verbose", "--tile-path", helloTilePath, "--ginkgo-flags", "-r -slowSpecThreshold 1"}) Expect(err).To(HaveOccurred()) By("logging helpful messages", func() { @@ -146,6 +164,94 @@ var _ = Describe("kiln test docker", func() { }) }) + When("stability tests should be successful", func() { + const ( + testSuccessLogLine = "manifest tests completed successfully" + ) + var fakeMobyClient *fakes.MobyClient + BeforeEach(func() { + fakeMobyClient = setupFakeMobyClient(testSuccessLogLine, 0) + }) + When("executing tests", func() { + var subjectUnderTest commands.TileTest + BeforeEach(func() { + writer.Reset() + subjectUnderTest = commands.NewTileTest(logger, ctx, fakeMobyClient, fakeSshProvider) + }) + When("verbose is passed", func() { + It("succeeds and logs info", func() { + err := subjectUnderTest.Execute([]string{"--stability", "--verbose", "--tile-path", helloTilePath, "--ginkgo-flags", "-r -slowSpecThreshold 1"}) + Expect(err).To(BeNil()) + + By("logging helpful messages", func() { + logs := writer.String() + By("logging container information", func() { + Expect(logs).To(ContainSubstring("Building / restoring cached docker image")) + }) + By("logging test lines", func() { + Expect(logs).To(ContainSubstring("manifest tests completed successfully")) + }) + }) + + By("creating a test container", func() { + Expect(fakeMobyClient.ContainerCreateCallCount()).To(Equal(1)) + _, config, _, _, _, _ := fakeMobyClient.ContainerCreateArgsForCall(0) + + By("executing the tests", func() { + expected := []string{ + "PRODUCT=hello-tile", + "RENDERER=ops-manifest", + "TAS_METADATA_PATH=/tas/hello-tile/test/manifest/fixtures/tas_metadata.yml", + "TAS_CONFIG_FILE=/tas/hello-tile/test/manifest/fixtures/tas_config.yml", + } + actual := config.Env + sort.Strings(actual) + sort.Strings(expected) + + dockerCmd := "cd /tas/hello-tile && ginkgo -r -slowSpecThreshold 1 /tas/hello-tile/test/stability" + Expect(config.Cmd).To(Equal(strslice.StrSlice{"/bin/bash", "-c", dockerCmd})) + }) + }) + }) + }) + When("verbose isn't passed", func() { + It("doesn't log info", func() { + err := subjectUnderTest.Execute([]string{"--stability", "--tile-path", helloTilePath, "--ginkgo-flags", "-r -slowSpecThreshold 1"}) + Expect(err).To(BeNil()) + + By("logging helpful messages", func() { + logs := writer.String() + By("logging container information", func() { + Expect(logs).NotTo(ContainSubstring("Building / restoring cached docker image")) + Expect(logs).NotTo(ContainSubstring("Info:")) + }) + By("logging test lines", func() { + Expect(logs).To(ContainSubstring("manifest tests completed successfully")) + }) + }) + }) + }) + }) + }) + + When("stability tests shouldn't be successful", func() { + const ( + testSuccessLogLine = "stability tests completed successfully" + ) + var fakeMobyClient *fakes.MobyClient + + BeforeEach(func() { + fakeMobyClient = setupFakeMobyClient(testSuccessLogLine, 0) + }) + + It("exits with an error if env vars are incorrectly formatted", func() { + subjectUnderTest := commands.NewTileTest(logger, ctx, fakeMobyClient, fakeSshProvider) + err := subjectUnderTest.Execute([]string{"--stability", "--verbose", "--tile-path", helloTilePath, "--ginkgo-flags", "-r -slowSpecThreshold 1", "--environment-variable", "MISFORMATTED_ENV_VAR"}) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("Environment variables must have the format [key]=[value]")) + }) + }) + When("all tests are run", func() { var fakeMobyClient *fakes.MobyClient BeforeEach(func() { @@ -167,7 +273,18 @@ var _ = Describe("kiln test docker", func() { _, config, _, _, _, _ := fakeMobyClient.ContainerCreateArgsForCall(0) By("executing the tests", func() { - dockerCmd := "cd /tas/hello-tile/test/manifest && PRODUCT=hello-tile RENDERER=ops-manifest ginkgo -r -p -slowSpecThreshold 15 && cd /tas/hello-tile/migrations && npm install && npm test" + dockerCmd := "cd /tas/hello-tile/migrations && npm install && npm test && cd /tas/hello-tile && ginkgo -r -p -slowSpecThreshold 15 /tas/hello-tile/test/stability /tas/hello-tile/test/manifest" + actual := config.Env + expected := []string{ + "TAS_CONFIG_FILE=/tas/hello-tile/test/manifest/fixtures/tas_config.yml", + "PRODUCT=hello-tile", + "RENDERER=ops-manifest", + "TAS_METADATA_PATH=/tas/hello-tile/test/manifest/fixtures/tas_metadata.yml", + } + + sort.Strings(expected) + sort.Strings(actual) + Expect(actual).To(Equal(expected)) Expect(config.Cmd).To(Equal(strslice.StrSlice{"/bin/bash", "-c", dockerCmd})) }) }) @@ -191,7 +308,7 @@ var _ = Describe("kiln test docker", func() { }) It("succeeds and logs info", func() { - err := subjectUnderTest.Execute([]string{"--migrations-only", "--verbose", "--tile-path", helloTilePath}) + err := subjectUnderTest.Execute([]string{"--migrations", "--verbose", "--tile-path", helloTilePath}) Expect(err).To(BeNil()) By("logging helpful messages", func() { @@ -228,6 +345,7 @@ var _ = Describe("kiln test docker", func() { Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("Docker daemon is not running")) }) + }) }) diff --git a/internal/commands/testdata/tas_fake/tas/test/stability/stability_test.go b/internal/commands/testdata/tas_fake/tas/test/stability/stability_test.go new file mode 100644 index 000000000..2155e4b5f --- /dev/null +++ b/internal/commands/testdata/tas_fake/tas/test/stability/stability_test.go @@ -0,0 +1,58 @@ +package stability + +import ( + "bytes" + _ "embed" + "os/exec" + "testing" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + "gopkg.in/yaml.v2" +) + +func TestFetcher(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Stability Suite") +} + +var _ = Describe("PropertyBlueprints", func() { + Context("hello-tile", func() { + It("doesn't have breaking changes", func() { + helloTilePatchMetadata, err := kilnBake(istBakeArgs()...) + Expect(err).To(BeNil()) + Expect(helloTilePatchMetadata.Name).To(Equal("example")) + }) + }) +}) + +func istBakeArgs() []string { + return []string{ + "--variables-file=variables/srt.yml", + "--variable=metadata-git-sha=develop", + "--stub-releases", + "--metadata-only", + } +} + +type metadata struct { + Name string `yaml:"name"` +} + +func kilnBake(args ...string) (metadata, error) { + cmd := exec.Command("kiln", append([]string{"bake"}, args...)...) + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + cmd.Dir = "../../" + err := cmd.Run() + if err != nil { + return metadata{}, err + } + var metadatas metadata + err = yaml.Unmarshal(stdout.Bytes(), &metadatas) + if err != nil { + return metadata{}, err + } + return metadatas, nil +} From 33d54bd49cc13f51696cdcc6a929b410be06a39d Mon Sep 17 00:00:00 2001 From: Nick Rohn Date: Wed, 21 Jun 2023 10:13:26 -0700 Subject: [PATCH 2/3] refactor: remove println Co-authored-by: Nick Rohn Co-authored-by: Gary Liu --- internal/commands/test_tile.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/commands/test_tile.go b/internal/commands/test_tile.go index 1158a37c1..381199c51 100644 --- a/internal/commands/test_tile.go +++ b/internal/commands/test_tile.go @@ -220,7 +220,6 @@ func (u TileTest) Execute(args []string) error { dockerCmd := strings.Join(dockerCmds, " && ") loggerWithInfo.Info("Running:", dockerCmd) envVars := getTileTestEnvVars(absRepoDir, tileDir, envMap) - fmt.Printf("%+v\n", envVarsToSlice(envVars)) createResp, err := u.mobi.ContainerCreate(u.ctx, &container.Config{ Image: "kiln_test_dependencies:vmware", Cmd: []string{"/bin/bash", "-c", dockerCmd}, From a159bd4507945e4c26c35399f19356051c785d42 Mon Sep 17 00:00:00 2001 From: Nick Rohn Date: Wed, 21 Jun 2023 10:15:16 -0700 Subject: [PATCH 3/3] feat: auto remove containers Co-authored-by: Nick Rohn Co-authored-by: Gary Liu --- internal/commands/test_tile.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/commands/test_tile.go b/internal/commands/test_tile.go index 381199c51..cf398e372 100644 --- a/internal/commands/test_tile.go +++ b/internal/commands/test_tile.go @@ -238,6 +238,7 @@ func (u TileTest) Execute(args []string) error { Target: "/tas", }, }, + AutoRemove: true, }, nil, nil, "") if err != nil { return err