From 5d5235807f9765e17924f573e69dd53690ca8c39 Mon Sep 17 00:00:00 2001 From: jmontesi Date: Tue, 23 Jan 2024 11:47:40 +0100 Subject: [PATCH] checksdb: turn the labels evaluator into a global variable The labels evaluator filtering mechanism is set at program startup and remains the same throughout its lifetime, so it seeems a better fit to have it as a global variable instead of recreating it every time it's needed. --- pkg/certsuite/certsuite.go | 9 +++++++-- pkg/checksdb/checksdb.go | 33 ++++++++++++++++++++------------- pkg/checksdb/checksgroup.go | 14 ++------------ pkg/checksdb/labels.go | 2 +- pkg/checksdb/labels_test.go | 2 +- 5 files changed, 31 insertions(+), 29 deletions(-) diff --git a/pkg/certsuite/certsuite.go b/pkg/certsuite/certsuite.go index 6eb8bc09b..b9527c580 100644 --- a/pkg/certsuite/certsuite.go +++ b/pkg/certsuite/certsuite.go @@ -93,9 +93,14 @@ func Run(labelsFilter, outputFolder string) error { timeout := processFlags() var returnErr bool + // Create an evaluator to filter test cases with labels + if err := checksdb.InitLabelsExprEvaluator(labelsFilter); err != nil { + return fmt.Errorf("failed to initialize a test case label evaluator, err: %v", err) + } + // If the list flag is passed, print the checks filtered with --labels and leave if *flags.ListFlag { - checksIDs, err := checksdb.FilterCheckIDs(labelsFilter) + checksIDs, err := checksdb.FilterCheckIDs() if err != nil { return fmt.Errorf("could not list test cases, err: %v", err) } @@ -120,7 +125,7 @@ func Run(labelsFilter, outputFolder string) error { log.Info("Running checks matching labels expr %q with timeout %v", labelsFilter, timeout) startTime := time.Now() - failedCtr, err := checksdb.RunChecks(labelsFilter, timeout) + failedCtr, err := checksdb.RunChecks(timeout) if err != nil { log.Error("%v", err) } diff --git a/pkg/checksdb/checksdb.go b/pkg/checksdb/checksdb.go index 4b181360d..12cf27bac 100644 --- a/pkg/checksdb/checksdb.go +++ b/pkg/checksdb/checksdb.go @@ -22,12 +22,14 @@ var ( dbByGroup map[string]*ChecksGroup resultsDB = map[string]claim.Result{} + + labelsExprEvaluator LabelsExprEvaluator ) type AbortPanicMsg string //nolint:funlen -func RunChecks(labelsExpr string, timeout time.Duration) (failedCtr int, err error) { +func RunChecks(timeout time.Duration) (failedCtr int, err error) { dbLock.Lock() defer dbLock.Unlock() @@ -45,8 +47,7 @@ func RunChecks(labelsExpr string, timeout time.Duration) (failedCtr int, err err var errs []error for _, group := range dbByGroup { if abort { - // ToDo: remove labelexpr checking. - _ = group.OnAbort(labelsExpr, abortReason) + _ = group.OnAbort(abortReason) group.RecordChecksResults() continue } @@ -58,7 +59,7 @@ func RunChecks(labelsExpr string, timeout time.Duration) (failedCtr int, err err // Done channel for the goroutine that runs group.RunChecks(). groupDone := make(chan bool) go func() { - checks, failedCheckCtr := group.RunChecks(labelsExpr, stopChan, abortChan) + checks, failedCheckCtr := group.RunChecks(stopChan, abortChan) failedCtr += failedCheckCtr errs = append(errs, checks...) groupDone <- true @@ -72,21 +73,21 @@ func RunChecks(labelsExpr string, timeout time.Duration) (failedCtr int, err err stopChan <- true abort = true - _ = group.OnAbort(labelsExpr, abortReason) + _ = group.OnAbort(abortReason) case <-timeOutChan: log.Warn("Running all checks timed-out.") stopChan <- true abort = true abortReason = "global time-out" - _ = group.OnAbort(labelsExpr, abortReason) + _ = group.OnAbort(abortReason) case <-sigIntChan: log.Warn("SIGINT/SIGTERM received.") stopChan <- true abort = true abortReason = "SIGINT/SIGTERM" - _ = group.OnAbort(labelsExpr, abortReason) + _ = group.OnAbort(abortReason) } group.RecordChecksResults() @@ -231,13 +232,8 @@ func GetTestsCountByState(state string) int { return count } -func FilterCheckIDs(labelsFilter string) ([]string, error) { +func FilterCheckIDs() ([]string, error) { filteredCheckIDs := []string{} - labelsExprEvaluator, err := NewLabelsExprEvaluator(labelsFilter) - if err != nil { - return nil, fmt.Errorf("invalid labels expression: %v", err) - } - for _, group := range dbByGroup { for _, check := range group.checks { if labelsExprEvaluator.Eval(check.Labels) { @@ -248,3 +244,14 @@ func FilterCheckIDs(labelsFilter string) ([]string, error) { return filteredCheckIDs, nil } + +func InitLabelsExprEvaluator(labelsFilter string) error { + eval, err := newLabelsExprEvaluator(labelsFilter) + if err != nil { + return fmt.Errorf("could not create a label evaluator, err: %v", err) + } + + labelsExprEvaluator = eval + + return nil +} diff --git a/pkg/checksdb/checksgroup.go b/pkg/checksdb/checksgroup.go index ea234d0ca..45e5ed837 100644 --- a/pkg/checksdb/checksgroup.go +++ b/pkg/checksdb/checksgroup.go @@ -293,15 +293,10 @@ func runCheck(check *Check, group *ChecksGroup, remainingChecks []*Check) (err e // - AfterEach panic: Set check as error. // //nolint:funlen -func (group *ChecksGroup) RunChecks(labelsExpr string, stopChan <-chan bool, abortChan chan string) (errs []error, failedChecks int) { +func (group *ChecksGroup) RunChecks(stopChan <-chan bool, abortChan chan string) (errs []error, failedChecks int) { log.Info("Running group %q checks.", group.name) fmt.Printf("Running suite %s\n", strings.ToUpper(group.name)) - labelsExprEvaluator, err := NewLabelsExprEvaluator(labelsExpr) - if err != nil { - return []error{fmt.Errorf("invalid labels expression: %v", err)}, 0 - } - // Get checks to run based on the label expr. checks := []*Check{} for _, check := range group.checks { @@ -382,12 +377,7 @@ func (group *ChecksGroup) RunChecks(labelsExpr string, stopChan <-chan bool, abo return errs, failedChecks } -func (group *ChecksGroup) OnAbort(labelsExpr, abortReason string) error { - labelsExprEvaluator, err := NewLabelsExprEvaluator(labelsExpr) - if err != nil { - return fmt.Errorf("invalid labels expression: %v", err) - } - +func (group *ChecksGroup) OnAbort(abortReason string) error { // If this wasn't the group with the aborted check. if group.currentRunningCheckIdx == checkIdxNone { fmt.Printf("Skipping checks from suite %s\n", strings.ToUpper(group.name)) diff --git a/pkg/checksdb/labels.go b/pkg/checksdb/labels.go index 8304fa606..8e50be581 100644 --- a/pkg/checksdb/labels.go +++ b/pkg/checksdb/labels.go @@ -18,7 +18,7 @@ type labelsExprParser struct { astRootNode ast.Expr } -func NewLabelsExprEvaluator(labelsExpr string) (LabelsExprEvaluator, error) { +func newLabelsExprEvaluator(labelsExpr string) (LabelsExprEvaluator, error) { goLikeExpr := strings.ReplaceAll(labelsExpr, "-", "_") goLikeExpr = strings.ReplaceAll(goLikeExpr, ",", "||") diff --git a/pkg/checksdb/labels_test.go b/pkg/checksdb/labels_test.go index 8611093f2..5795337a0 100644 --- a/pkg/checksdb/labels_test.go +++ b/pkg/checksdb/labels_test.go @@ -124,7 +124,7 @@ func TestIsWordInExpr(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { // Get labels expr evaluator - labelsExprEvaluator, err := NewLabelsExprEvaluator(tt.args.expr) + labelsExprEvaluator, err := newLabelsExprEvaluator(tt.args.expr) assert.NotNil(t, labelsExprEvaluator) assert.Nil(t, err)