Ensure ability to run w/out partition when partition configured (#49)

rwx-research · Apr 26, 2023 · c61c8dc · c61c8dc
1 parent 149a1c9
commit c61c8dc
Show file tree

Hide file tree

Showing 4 changed files with 52 additions and 15 deletions.
diff --git a/internal/cli/config.go b/internal/cli/config.go
@@ -137,8 +137,7 @@ func (rc RunConfig) MaxTestsToRetryPercentage() (*float64, error) {
 
 func (rc RunConfig) IsRunningPartition() bool {
 	// TODO: Should we have a bit somewhere that indicates provider defaulted?
-	return rc.PartitionCommandTemplate != "" &&
-		(rc.PartitionConfig.PartitionNodes.Index != -1 || rc.PartitionConfig.PartitionNodes.Total != -1)
+	return rc.PartitionCommandTemplate != "" && rc.PartitionConfig.PartitionNodes.Total >= 1
 }
 
 type PartitionConfig struct {

diff --git a/internal/cli/config_test.go b/internal/cli/config_test.go
@@ -144,6 +144,35 @@ var _ = Describe("RunConfig", func() {
 			Expect(err.Error()).To(ContainSubstring("Unsupported partitioning setup"))
 		})
 
+		It("errs when partitioning and partition config has nonsense index", func() {
+			err := cli.RunConfig{
+				PartitionCommandTemplate: "something {{ testFiles }}",
+				PartitionConfig: cli.PartitionConfig{
+					SuiteID: "your-suite",
+					PartitionNodes: config.PartitionNodes{
+						Index: -1,
+						Total: 1,
+					},
+				},
+			}.Validate()
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(ContainSubstring("Missing partition index"))
+		})
+
+		It("is not an error when partitioning is configured but not currently partitioning", func() {
+			err := cli.RunConfig{
+				PartitionCommandTemplate: "something {{ testFiles }}",
+				PartitionConfig: cli.PartitionConfig{
+					SuiteID: "your-suite",
+					PartitionNodes: config.PartitionNodes{
+						Index: -1,
+						Total: -1,
+					},
+				},
+			}.Validate()
+			Expect(err).NotTo(HaveOccurred())
+		})
+
 		It("is valid when partitioning and partition config has command and globs", func() {
 			err := cli.RunConfig{
 				PartitionCommandTemplate: "something {{ testFiles }}",
@@ -205,7 +234,7 @@ var _ = Describe("RunConfig", func() {
 			Expect(cli.RunConfig{}.IsRunningPartition()).To(Equal(false))
 		})
 
-		It("returns false when partition command template is set, but neither partition index nor total are set", func() {
+		It("returns false when partition command template is set, but partition nodes are defaulted to -1", func() {
 			rc := cli.RunConfig{
 				PartitionCommandTemplate: "bin/rspec {{testFiles}}",
 				PartitionConfig: cli.PartitionConfig{
@@ -218,23 +247,20 @@ var _ = Describe("RunConfig", func() {
 			Expect(rc.IsRunningPartition()).To(Equal(false))
 		})
 
-		It("returns true when partition command template and partition index is set", func() {
+		It("returns false when partition command template is set, but neither partition nodes are unset", func() {
 			rc := cli.RunConfig{
 				PartitionCommandTemplate: "bin/rspec {{testFiles}}",
-				PartitionConfig: cli.PartitionConfig{
-					PartitionNodes: config.PartitionNodes{
-						Index: 0,
-					},
-				},
+				PartitionConfig:          cli.PartitionConfig{},
 			}
-			Expect(rc.IsRunningPartition()).To(Equal(true))
+			Expect(rc.IsRunningPartition()).To(Equal(false))
 		})
 
 		It("returns true when partition command template and partition total is set", func() {
 			rc := cli.RunConfig{
 				PartitionCommandTemplate: "bin/rspec {{testFiles}}",
 				PartitionConfig: cli.PartitionConfig{
 					PartitionNodes: config.PartitionNodes{
+						Index: 0,
 						Total: 2,
 					},
 				},

diff --git a/internal/providers/provider.go b/internal/providers/provider.go
@@ -90,11 +90,7 @@ func Merge(into Provider, from Provider) Provider {
 		into.CommitMessage = firstNonempty(from.CommitMessage, into.CommitMessage)
 		into.Title = firstNonempty(from.Title, into.Title)
 
-		intoPartitionImplicitlyUnset := into.PartitionNodes == config.PartitionNodes{}
-		intoPartitionExplicitlyUnset := into.PartitionNodes.Index < 0 || into.PartitionNodes.Total < 0
-		fromPartitionExplicitlySet := from.PartitionNodes.Index >= 0 && from.PartitionNodes.Total >= 1
-
-		if fromPartitionExplicitlySet && (intoPartitionExplicitlyUnset || intoPartitionImplicitlyUnset) {
+		if into.PartitionNodes.Total <= 0 && from.PartitionNodes.Total >= 1 {
 			into.PartitionNodes = from.PartitionNodes
 		}
 

diff --git a/test/oss_integration_test.go b/test/oss_integration_test.go
@@ -779,6 +779,22 @@ var _ = Describe(versionedPrefixForQuarantining()+"OSS mode Integration Tests",
 				Expect(result.exitCode).To(Equal(0))
 			})
 		})
+
+		Context("when partitioning is configured but flags aren't provided", func() {
+			It("runs the default run command", func() {
+				result := runCaptain(captainArgs{
+					args: []string{
+						"run",
+						"oss-run-with-partition",
+						"--config-file", "fixtures/integration-tests/partition-config.yaml",
+					},
+					env: make(map[string]string),
+				})
+
+				Expect(result.exitCode).To(Equal(123))
+				Expect(result.stdout).To(Equal("default run"))
+			})
+		})
 	})
 
 	Describe("captain run w/ partition", func() {