Checker: a package with skipped tests is withheld from the kept verdicts, not the whole result (issue 890)

modules/mesh-delivery-checker/internal/check/kept.go

@@ -56,18 +56,22 @@ func (k Kept) Keep(ctx context.Context, level, digest string, v delivery.Kept) (
 
 // KeepPasses keeps, for the packages of a test result that passed with tests, a verdict under the digest the build module
 // named for the package at the level. A package that failed, has no tests or has no digest is never kept (ADR 0444 clause
-// 1): it runs every check. A result in which any test skipped keeps nothing: the build module counts skips per result,
-// not per package, so no package of it is known to have run whole, and a pass that skipped (a test needing a toolchain the
-// node lacks) would otherwise be reused for its digest and never run anywhere (issue 890). It answers the packages it
-// kept now.
+// 1): it runs every check. A package with skipped tests is not kept either: a pass that skipped (a test needing a
+// toolchain the node lacks) would be reused for its digest and never run anywhere (issue 890). A build module that
+// counts skips only per result, not per package, keeps nothing of a result in which a test skipped. It answers the
+// packages it kept now.
 func (k Kept) KeepPasses(ctx context.Context, repository, level, toolchain string, res Tested, digests map[string]string, at time.Time) ([]string, error) {
 	var kept []string
-	if res.Skipped > 0 {
-		return kept, nil
+	perPackage := false
+	for _, p := range res.Packages {
+		perPackage = perPackage || p.Skipped > 0
+	}
+	if res.Skipped > 0 && !perPackage {
+		return kept, nil // an older build module: the skip cannot be placed in a package
 	}
 	for _, p := range res.Packages {
 		digest, ok := digests[p.Package]
-		if !ok || !p.Passed || p.NoTests || p.Tests == 0 {
+		if !ok || !p.Passed || p.NoTests || p.Tests == 0 || p.Skipped > 0 {
 			continue
 		}
 		if failedIn(res, p.Package) {

modules/mesh-delivery-checker/internal/check/kept_test.go

@@ -87,6 +87,19 @@ func TestAResultWithSkippedTestsKeepsNothing(t *testing.T) { // issue 890: a pas
 	}
 }
 
+func TestOnlyTheSkippingPackageOfAResultIsWithheld(t *testing.T) { // issue 890
+	ctx, st := context.Background(), newMemKV()
+	k := Kept{Store: st}
+	res := Tested{Level: "integration", Passed: true, Skipped: 2, Packages: []GoPackage{
+		{Package: "m/a", Passed: true, Tests: 4, Skipped: 2},
+		{Package: "m/b", Passed: true, Tests: 3},
+	}}
+	kept, err := k.KeepPasses(ctx, "novox/mesh-catalog", "integration", "go1.26.0", res, map[string]string{"m/a": digA, "m/b": digB}, at0)
+	if err != nil || strings.Join(kept, ",") != "m/b" {
+		t.Fatalf("kept %v, %v, want only m/b", kept, err)
+	}
+}
+
 func TestAKeptValueOfAnotherDigestIsNotRead(t *testing.T) { // 1287/AC7
 	ctx, st := context.Background(), newMemKV()
 	k := Kept{Store: st}

modules/mesh-delivery-checker/internal/check/results.go

@@ -52,6 +52,8 @@ type GoPackage struct {
 	Elapsed float64 `json:"elapsed"`
 	Tests   int     `json:"tests"`
 	NoTests bool    `json:"no_tests,omitempty"`
+	// Skipped is how many of the package's tests skipped; an older build module names none.
+	Skipped int `json:"skipped,omitempty"`
 }
 
 // Failure is a test that failed, or a package that failed with no test to name.