[rayci] add option noTagMeansAlways (#266)

and set as false by default. when it is set to false, this will shake
off unnecessary steps that have no tags when tag based conditional
testing is enabled.

this is a change on the default behavior.

Author: aslonnie

raycicmd/config.go

@@ -146,6 +146,11 @@ type config struct {
 	//
 	// Optional.
 	DockerPlugin *dockerPluginConfig `yaml:"docker_plugin"`
+
+	// NoTagMeansAlways sets if a step without any tags should be treated as
+	// a step with a magic "always" tag, that will always be picked during the
+	// conditional testing tag picking phase.
+	NoTagMeansAlways bool `yaml:"no_tag_means_always"`
 }
 
 func builderAgent(config *config, instanceType string) string {

raycicmd/converter_test.go

@@ -751,6 +751,8 @@ func TestConvertPipelineGroup(t *testing.T) {
 	filter := &stepFilter{
 		skipTags: stringSet("disabled"),
 		tags:     stringSet("foo"),
+
+		noTagMeansAlways: true,
 	}
 	bk, err := convertSingleGroup(c, g, filter)
 	if err != nil {

raycicmd/make.go

@@ -99,6 +99,8 @@ func makePipeline(repoDir string, config *config, info *buildInfo) (
 		return nil, fmt.Errorf("run tag filter command: %w", err)
 	}
 
+	filter.noTagMeansAlways = config.NoTagMeansAlways
+
 	// Build steps for CI.
 	bkDirs := config.BuildkiteDirs
 	if len(bkDirs) == 0 {

raycicmd/make_test.go

@@ -225,7 +225,6 @@ func TestMakePipeline(t *testing.T) {
 		SkipTags:      []string{"disabled"},
 		BuildkiteDirs: []string{".buildkite", "private/buildkite"},
 	}
-
 	t.Run("filter", func(t *testing.T) {
 		config := *commonConfig
 		config.TagFilterCommand = []string{"echo", "enabled"}
@@ -240,6 +239,41 @@ func TestMakePipeline(t *testing.T) {
 			t.Fatalf("makePipeline: %v", err)
 		}
 
+		// Should only select the enabled steps and its dependencies.
+		// which are 2 steps in 2 different groups.
+		if want := 2; len(got.Steps) != want {
+			t.Errorf("got %d groups, want %d", len(got.Steps), want)
+		}
+
+		// sub funtions are already tested in their unit tests.
+		// so we only check the total number of groups here.
+		// we also have an e2e test at the repo level.
+
+		totalSteps := 0
+		for _, g := range got.Steps {
+			totalSteps += len(g.Steps)
+		}
+
+		if want := 2; totalSteps != want {
+			t.Fatalf("got %d steps, want %d", totalSteps, want)
+		}
+	})
+
+	t.Run("filter_noTagMeansAlways", func(t *testing.T) {
+		config := *commonConfig
+		config.TagFilterCommand = []string{"echo", "enabled"}
+		config.NoTagMeansAlways = true
+
+		buildID := "fakebuild"
+		info := &buildInfo{
+			buildID: buildID,
+		}
+
+		got, err := makePipeline(tmp, &config, info)
+		if err != nil {
+			t.Fatalf("makePipeline: %v", err)
+		}
+
 		if want := 3; len(got.Steps) != want { // all steps are groups.
 			t.Errorf("got %d groups, want %d", len(got.Steps), want)
 		}

raycicmd/step_filter.go

@@ -18,6 +18,8 @@ type stepFilter struct {
 	// third pass: selecting steps
 	selects    map[string]bool // based on ID or key
 	tagSelects map[string]bool // or based on tags
+
+	noTagMeansAlways bool
 }
 
 func (f *stepFilter) reject(step *stepNode) bool {
@@ -42,9 +44,11 @@ func (f *stepFilter) acceptTagHit(step *stepNode) bool {
 		return true
 	}
 
-	// if not in run-all mode, hit when the step has any of the tags.
-	if !step.hasTags() {
-		return true // step does not have any tags: a step that always runs
+	if f.noTagMeansAlways {
+		// if noTagMeansAlways is set, hit when the step has any of the tags.
+		if !step.hasTags() {
+			return true // step does not have any tags: a step that always runs
+		}
 	}
 	return step.hasTagInMap(f.tags)
 }

raycicmd/step_filter_test.go

@@ -64,6 +64,8 @@ func TestStepFilter_tags(t *testing.T) {
 	filter := &stepFilter{
 		skipTags: stringSet("disabled"),
 		tags:     stringSet("tune"),
+
+		noTagMeansAlways: true,
 	}
 
 	for _, tags := range [][]string{
@@ -217,12 +219,13 @@ func TestStepFilter_tagSelects(t *testing.T) {
 	}
 }
 
-func TestStepFilter_selectsAndTags(t *testing.T) {
+func TestStepFilter_selectsAndTags_noTagMeansAlways(t *testing.T) {
 	filter, _ := newStepFilter(
 		[]string{"disabled"},
 		[]string{"foo", "bar", "tag:pick"},
 		[]string{"echo", "tune"},
 	)
+	filter.noTagMeansAlways = true
 
 	for _, node := range []*stepNode{
 		{key: "foo"},
@@ -246,6 +249,35 @@ func TestStepFilter_selectsAndTags(t *testing.T) {
 	}
 }
 
+func TestStepFilter_selectsAndTags(t *testing.T) {
+	filter, _ := newStepFilter(
+		[]string{"disabled"},
+		[]string{"foo", "bar", "tag:pick"},
+		[]string{"echo", "tune"},
+	)
+
+	for _, node := range []*stepNode{
+		{id: "foo", tags: []string{"tune"}},
+		{id: "other", tags: []string{"pick", "tune"}},
+	} {
+		if !filter.accept(node) {
+			t.Errorf("miss %+v", node)
+		}
+	}
+
+	for _, node := range []*stepNode{
+		{key: "foo"}, // need to have tags
+		{id: "bar"},  // need to have tags
+		{id: "foo", tags: []string{"not_tune"}},
+		{id: "bar", tags: []string{"tune_not"}},
+		{key: "w00t"},
+	} {
+		if filter.accept(node) {
+			t.Errorf("hit %+v", node)
+		}
+	}
+}
+
 func TestRunFilterCmd(t *testing.T) {
 	for _, test := range []struct {
 		cmd []string