Split failing and passing tests in the summary. (CP of #7835) (#7764)

Author: PawelWMS

toolkit/tools/scheduler/schedulerutils/buildworker.go

@@ -51,6 +51,7 @@ type BuildResult struct {
 	Err            error               // Error encountered during the build.
 	LogFile        string              // Path to the log file from the build.
 	Node           *pkggraph.PkgNode   // The main node being analyzed for the build.
+	CheckFailed    bool                // Indicator if the package test failed but the build itself was correct.
 	Ignored        bool                // Indicator if the build was ignored by user request.
 	UsedCache      bool                // Indicator if we used the cached artifacts (external or earlier local build) instead of building the node.
 	WasDelta       bool                // Indicator if we used a pre-built component from an external repository instead of building the node.
@@ -120,7 +121,7 @@ func BuildNodeWorker(channels *BuildChannels, agent buildagents.BuildAgent, grap
 			}
 
 		case pkggraph.TypeTest:
-			res.Ignored, res.LogFile, res.Err = testNode(req, graphMutex, agent, checkAttempts, ignoredTests)
+			res.CheckFailed, res.Ignored, res.LogFile, res.Err = testNode(req, graphMutex, agent, checkAttempts, ignoredTests)
 			if res.Err == nil {
 				setAncillaryBuildNodesStatus(req, graphMutex, pkggraph.StateUpToDate)
 			} else {
@@ -175,7 +176,7 @@ func buildNode(request *BuildRequest, graphMutex *sync.RWMutex, agent buildagent
 }
 
 // testNode tests a TypeTest node.
-func testNode(request *BuildRequest, graphMutex *sync.RWMutex, agent buildagents.BuildAgent, checkAttempts int, ignoredTests []*pkgjson.PackageVer) (ignored bool, logFile string, err error) {
+func testNode(request *BuildRequest, graphMutex *sync.RWMutex, agent buildagents.BuildAgent, checkAttempts int, ignoredTests []*pkgjson.PackageVer) (checkFailed, ignored bool, logFile string, err error) {
 	node := request.Node
 	baseSrpmName := node.SRPMFileName()
 
@@ -200,7 +201,7 @@ func testNode(request *BuildRequest, graphMutex *sync.RWMutex, agent buildagents
 	dependencies := getBuildDependencies(node, request.PkgGraph, graphMutex)
 
 	logger.Log.Infof("Testing: %s", baseSrpmName)
-	logFile, err = testSRPMFile(agent, checkAttempts, basePackageName, node.SrpmPath, node.Architecture, dependencies)
+	logFile, checkFailed, err = testSRPMFile(agent, checkAttempts, basePackageName, node.SrpmPath, node.Architecture, dependencies)
 	return
 }
 
@@ -239,7 +240,7 @@ func getBuildDependencies(node *pkggraph.PkgNode, pkgGraph *pkggraph.PkgGraph, g
 }
 
 // parseCheckSection reads the package build log file to determine if the %check section passed or not
-func parseCheckSection(logFile string) (err error) {
+func parseCheckSection(logFile string) (checkFailed bool, err error) {
 	logFileObject, err := os.Open(logFile)
 	// If we can't open the log file, that's a build error.
 	if err != nil {
@@ -261,7 +262,7 @@ func parseCheckSection(logFile string) (err error) {
 				logger.Log.Errorf("Log file copy failed. Error: %v", err)
 				return
 			}
-			err = fmt.Errorf("package test failed. Test status line: %s", currLine)
+			checkFailed = true
 			return
 		}
 	}
@@ -285,14 +286,14 @@ func buildSRPMFile(agent buildagents.BuildAgent, buildAttempts int, basePackageN
 }
 
 // testSRPMFile sends an SRPM to a build agent to test.
-func testSRPMFile(agent buildagents.BuildAgent, checkAttempts int, basePackageName string, srpmFile string, outArch string, dependencies []string) (logFile string, err error) {
+// The 'checkFailed' flag says if the package test failed as opposed
+// to the build failing for another reason, which is reflected by a non-nil 'err'.
+func testSRPMFile(agent buildagents.BuildAgent, checkAttempts int, basePackageName string, srpmFile string, outArch string, dependencies []string) (logFile string, checkFailed bool, err error) {
 	const (
 		retryDuration = time.Second
 		runCheck      = true
 	)
 
-	// checkFailed is a flag to see if a non-null buildErr is from the %check section
-	checkFailed := false
 	logBaseName := filepath.Base(srpmFile) + ".test.log"
 	err = retry.Run(func() (buildErr error) {
 		checkFailed = false
@@ -303,13 +304,16 @@ func testSRPMFile(agent buildagents.BuildAgent, checkAttempts int, basePackageNa
 			return
 		}
 
-		buildErr = parseCheckSection(logFile)
-		checkFailed = (buildErr != nil)
+		checkFailed, buildErr = parseCheckSection(logFile)
+		// If the build succeeded but tests failed, we still want to retry.
+		if buildErr == nil && checkFailed {
+			buildErr = fmt.Errorf("package test for '%s' failed", basePackageName)
+		}
 		return
 	}, checkAttempts, retryDuration)
 
-	if err != nil && checkFailed {
-		logger.Log.Warnf("Tests failed for '%s'. Error: %s", srpmFile, err)
+	if checkFailed {
+		logger.Log.Debugf("Tests failed for '%s' after %d retries.", basePackageName, checkAttempts)
 		err = nil
 	}
 	return

toolkit/tools/scheduler/schedulerutils/graphbuildstate.go

@@ -194,7 +194,8 @@ func (g *GraphBuildState) RecordBuildResult(res *BuildResult, allowToolchainRebu
 
 	delete(g.activeBuilds, res.Node.ID())
 
-	if res.Err != nil {
+	failure := (res.Err != nil) || res.CheckFailed
+	if failure {
 		g.failures = append(g.failures, res)
 	}
 
@@ -208,7 +209,7 @@ func (g *GraphBuildState) RecordBuildResult(res *BuildResult, allowToolchainRebu
 	}
 
 	state := &nodeState{
-		available: res.Err == nil,
+		available: !failure,
 		cached:    res.UsedCache,
 		usedDelta: res.WasDelta,
 		freshness: freshness,

toolkit/tools/scheduler/schedulerutils/printresults.go

@@ -41,6 +41,8 @@ func PrintBuildResult(res *BuildResult) {
 			logger.Log.Warnf("Ignored test for '%s' per user request.", baseSRPMName)
 		} else if res.UsedCache {
 			logger.Log.Infof("Skipped test: %s", baseSRPMName)
+		} else if res.CheckFailed {
+			logger.Log.Warnf("Failed test: %s", baseSRPMName)
 		} else {
 			logger.Log.Infof("Tested: %s", baseSRPMName)
 		}
@@ -145,7 +147,7 @@ func PrintBuildSummary(pkgGraph *pkggraph.PkgGraph, graphMutex *sync.RWMutex, bu
 	}
 
 	if len(testedSRPMs) != 0 {
-		logger.Log.Info(color.GreenString("Tested SRPMs:"))
+		logger.Log.Info(color.GreenString("Passed SRPMs tests:"))
 		keys := mapToSortedSlice(testedSRPMs)
 		for _, testedSRPM := range keys {
 			logger.Log.Infof("--> %s", filepath.Base(testedSRPM))
@@ -206,7 +208,7 @@ func PrintBuildSummary(pkgGraph *pkggraph.PkgGraph, graphMutex *sync.RWMutex, bu
 		keys := mapToSortedSlice(failedSRPMsTests)
 		for _, key := range keys {
 			failure := failedSRPMsTests[key]
-			logger.Log.Infof("--> %s , error: %s, for details see: %s", failure.Node.SRPMFileName(), failure.Err, failure.LogFile)
+			logger.Log.Infof("--> %s , for details see: %s", failure.Node.SRPMFileName(), failure.LogFile)
 		}
 	}
 
@@ -344,7 +346,7 @@ func printSummary(failedSRPMs, failedSRPMsTests map[string]*BuildResult, prebuil
 	logger.Log.Infof(color.GreenString(summaryLine("Number of prebuilt delta SRPMs:", len(prebuiltDeltaSRPMs))))
 	logger.Log.Infof(color.GreenString(summaryLine("Number of skipped SRPMs tests:", len(skippedSRPMsTests))))
 	logger.Log.Infof(color.GreenString(summaryLine("Number of built SRPMs:", len(builtSRPMs))))
-	logger.Log.Infof(color.GreenString(summaryLine("Number of tested SRPMs:", len(testedSRPMs))))
+	logger.Log.Infof(color.GreenString(summaryLine("Number of passed SRPMs tests:", len(testedSRPMs))))
 	printErrorInfoByCondition(len(unresolvedDependencies) > 0, summaryLine("Number of unresolved dependencies:", len(unresolvedDependencies)))
 	printErrorInfoByCondition(len(blockedSRPMs) > 0, summaryLine("Number of blocked SRPMs:", len(blockedSRPMs)))
 	printErrorInfoByCondition(len(blockedSRPMsTests) > 0, summaryLine("Number of blocked SRPMs tests:", len(blockedSRPMsTests)))