test_planner: Don't test non-critical builds

This may not be desirable long-term behavior, but it unblocks our
enabling of the full suite of CQ builders for P-CQ.

BUG=None
TEST=Unit tests

Change-Id: I9f286a0676107a8d51dc9eedc99f0271900fc511
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/infra/test_planner/+/1614275
Reviewed-by: Dhanya Ganesh <dhanyaganesh@chromium.org>
Tested-by: Sean Abraham <seanabraham@chromium.org>
Commit-Queue: Sean Abraham <seanabraham@chromium.org>
diff --git a/deploy_cipd.json b/deploy_cipd.json
index cc680de..2c3bf3c 100644
--- a/deploy_cipd.json
+++ b/deploy_cipd.json
@@ -1,6 +1,6 @@
 {
   "result": {
     "package": "chromiumos/infra/test_planner",
-    "instance_id": "k6R1yB6jhcFt51tF1ymGiFC6lTH1huyLQ4B4pW8x4XUC"
+    "instance_id": "-hIcDWKd2u00yTupEaPY3RiTiSpPvYi-ri1Zs87JGZQC"
   }
 }
\ No newline at end of file
diff --git a/src/testplans/internal/generator/generator.go b/src/testplans/internal/generator/generator.go
index ea3ec1f..d05a0b6 100644
--- a/src/testplans/internal/generator/generator.go
+++ b/src/testplans/internal/generator/generator.go
@@ -137,7 +137,12 @@
 		pttr := tbr.perTargetTestReqs
 		bt := chromiumos.BuildTarget{Name: string(tbr.buildTarget)}
 		tuc := &testplans.TestUnitCommon{BuildTarget: &bt, BuildPayload: bp}
-		critical := &wrappers.BoolValue{Value: tbr.buildReport.Critical != bbproto.Trinary_NO}
+		isCritical := tbr.buildReport.Critical != bbproto.Trinary_NO
+		if !isCritical {
+			log.Printf("Build target %s is not critical. Skipping...", tbr.buildTarget)
+			continue targetLoop
+		}
+		critical := &wrappers.BoolValue{Value: isCritical}
 		if pttr.GceTestCfg != nil {
 			for _, gce := range pttr.GceTestCfg.GceTest {
 				gce.Common = withCritical(gce.Common, critical)
diff --git a/src/testplans/internal/generator/generator_test.go b/src/testplans/internal/generator/generator_test.go
index dbd279e..dbdbe86 100644
--- a/src/testplans/internal/generator/generator_test.go
+++ b/src/testplans/internal/generator/generator_test.go
@@ -98,7 +98,7 @@
 	bbBuilds := []*bbproto.Build{
 		makeBuildbucketBuild("kevin", bbproto.Status_SUCCESS, []*bbproto.GerritChange{
 			{Host: "test-review.googlesource.com", Change: 123, Patchset: 2},
-		}, false),
+		}, true),
 		makeBuildbucketBuild("reef", bbproto.Status_SUCCESS, []*bbproto.GerritChange{
 			{Host: "test-review.googlesource.com", Change: 123, Patchset: 2},
 		}, true),
@@ -413,3 +413,52 @@
 		t.Errorf("expected no error, but got %v", err)
 	}
 }
+
+func TestCreateCombinedTestPlan_skipsNonCritical(t *testing.T) {
+	// In this test, the build is not critical, so no test unit will be produced.
+
+	reefGceTestCfg := &testplans.GceTestCfg{GceTest: []*testplans.GceTestCfg_GceTest{
+		{TestType: "GCE reef"},
+	}}
+	testReqs := &testplans.TargetTestRequirementsCfg{
+		PerTargetTestRequirements: []*testplans.PerTargetTestRequirements{
+			{TargetCriteria: &testplans.TargetCriteria{
+				TargetType: &testplans.TargetCriteria_BuildTarget{BuildTarget: "reef"}},
+				GceTestCfg: reefGceTestCfg},
+		},
+	}
+	sourceTreeTestCfg := &testplans.SourceTreeTestCfg{
+		SourceTreeTestRestriction: []*testplans.SourceTreeTestRestriction{
+			{SourceTree: &testplans.SourceTree{Path: "hw/tests/not/needed/here"},
+				TestRestriction: &testplans.TestRestriction{DisableHwTests: true}}}}
+	bbBuilds := []*bbproto.Build{
+		makeBuildbucketBuild("reef", bbproto.Status_SUCCESS, []*bbproto.GerritChange{
+			{Host: "test-review.googlesource.com", Change: 123, Patchset: 2},
+		}, false),
+	}
+	chRevData := git.GetChangeRevsForTest([]*git.ChangeRev{
+		{
+			ChangeRevKey: git.ChangeRevKey{
+				Host:      "test-review.googlesource.com",
+				ChangeNum: 123,
+				Revision:  2,
+			},
+			Project: "chromiumos/repo/name",
+			Files:   []string{"a/b/c"},
+		},
+	})
+	repoToSrcRoot := map[string]string{"chromiumos/repo/name": "src/to/file"}
+
+	actualTestPlan, err := CreateTestPlan(testReqs, sourceTreeTestCfg, bbBuilds, chRevData, repoToSrcRoot)
+	if err != nil {
+		t.Error(err)
+	}
+
+	expectedTestPlan := &testplans.GenerateTestPlanResponse{
+		TestUnit:     []*testplans.TestUnit{},
+		GceTestUnits: []*testplans.GceTestUnit{}}
+
+	if diff := cmp.Diff(expectedTestPlan, actualTestPlan, cmpopts.EquateEmpty()); diff != "" {
+		t.Errorf("CreateCombinedTestPlan bad result (-want/+got)\n%s", diff)
+	}
+}