diff --git a/toolkit/tools/scheduler/schedulerutils/buildworker.go b/toolkit/tools/scheduler/schedulerutils/buildworker.go index 936693e4f18..8ab9cb5791e 100644 --- a/toolkit/tools/scheduler/schedulerutils/buildworker.go +++ b/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 diff --git a/toolkit/tools/scheduler/schedulerutils/graphbuildstate.go b/toolkit/tools/scheduler/schedulerutils/graphbuildstate.go index 1401bfc4e4a..138124dacc6 100644 --- a/toolkit/tools/scheduler/schedulerutils/graphbuildstate.go +++ b/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, diff --git a/toolkit/tools/scheduler/schedulerutils/printresults.go b/toolkit/tools/scheduler/schedulerutils/printresults.go index 1fe6af328a8..8431b556b84 100644 --- a/toolkit/tools/scheduler/schedulerutils/printresults.go +++ b/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)))