You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by wu...@apache.org on 2022/11/14 15:22:21 UTC

[skywalking-infra-e2e] branch main updated: Add `batchOutput` config to reduce outputs (#92)

This is an automated email from the ASF dual-hosted git repository.

wusheng pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/skywalking-infra-e2e.git


The following commit(s) were added to refs/heads/main by this push:
     new f33db90  Add `batchOutput` config to reduce outputs (#92)
f33db90 is described below

commit f33db90d30ffb361fb0824be82abd6e7d4393ae1
Author: Hoshea Jiang <fg...@gmail.com>
AuthorDate: Mon Nov 14 23:22:15 2022 +0800

    Add `batchOutput` config to reduce outputs (#92)
---
 commands/root.go          |   5 +-
 commands/verify/verify.go | 257 +++++++++++++++++++---------------------------
 internal/util/config.go   |   7 +-
 pkg/output/printer.go     | 141 +++++++++++++++++++++++++
 4 files changed, 255 insertions(+), 155 deletions(-)

diff --git a/commands/root.go b/commands/root.go
index 29572f7..4e3b463 100644
--- a/commands/root.go
+++ b/commands/root.go
@@ -23,8 +23,6 @@ import (
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 
-	"github.com/apache/skywalking-infra-e2e/internal/logger"
-
 	"github.com/apache/skywalking-infra-e2e/commands/cleanup"
 	"github.com/apache/skywalking-infra-e2e/commands/run"
 	"github.com/apache/skywalking-infra-e2e/commands/setup"
@@ -32,6 +30,7 @@ import (
 	"github.com/apache/skywalking-infra-e2e/commands/verify"
 	"github.com/apache/skywalking-infra-e2e/internal/config"
 	"github.com/apache/skywalking-infra-e2e/internal/constant"
+	"github.com/apache/skywalking-infra-e2e/internal/logger"
 	"github.com/apache/skywalking-infra-e2e/internal/util"
 )
 
@@ -94,6 +93,8 @@ func Execute() error {
 	Root.PersistentFlags().StringVarP(&util.WorkDir, "work-dir", "w", "~/.skywalking-infra-e2e", "the working directory for skywalking-infra-e2e")
 	Root.PersistentFlags().StringVarP(&util.LogDir, "log-dir", "l", "~/.skywalking-infra-e2e/logs", "the container logs directory for environment")
 	Root.PersistentFlags().StringVarP(&util.CfgFile, "config", "c", constant.E2EDefaultFile, "the config file")
+	Root.PersistentFlags().BoolVarP(&util.BatchMode, "batch-mode", "B", true,
+		"whether to output in batch mode, if false, the output will be printed in real time. This option is not valid in concurrency mode.")
 
 	return Root.Execute()
 }
diff --git a/commands/verify/verify.go b/commands/verify/verify.go
index e2061b1..b1e3296 100644
--- a/commands/verify/verify.go
+++ b/commands/verify/verify.go
@@ -18,23 +18,25 @@
 package verify
 
 import (
+	"context"
 	"fmt"
 	"sync"
 	"time"
 
+	"github.com/spf13/cobra"
+
 	"github.com/apache/skywalking-infra-e2e/internal/components/verifier"
 	"github.com/apache/skywalking-infra-e2e/internal/config"
 	"github.com/apache/skywalking-infra-e2e/internal/logger"
 	"github.com/apache/skywalking-infra-e2e/internal/util"
-
-	"github.com/pterm/pterm"
-	"github.com/spf13/cobra"
+	"github.com/apache/skywalking-infra-e2e/pkg/output"
 )
 
 var (
 	query    string
 	actual   string
 	expected string
+	printer  output.Printer
 )
 
 func init() {
@@ -51,6 +53,7 @@ var Verify = &cobra.Command{
 		if expected != "" {
 			return verifySingleCase(expected, actual, query)
 		}
+
 		// If there is no given flags.
 		return DoVerifyAccordingConfig()
 	},
@@ -64,21 +67,6 @@ type verifyInfo struct {
 	failFast   bool
 }
 
-type Summary struct {
-	errNum     int
-	successNum int
-}
-
-type CaseInfo struct {
-	msg string
-	err error
-}
-
-type OutputInfo struct {
-	writeLock sync.Mutex
-	casesInfo []CaseInfo
-}
-
 func verifySingleCase(expectedFile, actualFile, query string) error {
 	expectedData, err := util.ReadFileContent(expectedFile)
 	if err != nil {
@@ -109,174 +97,156 @@ func verifySingleCase(expectedFile, actualFile, query string) error {
 	return nil
 }
 
-func concurrentlyVerifySingleCase(v *config.VerifyCase, verifyInfo *verifyInfo, wg *sync.WaitGroup, outputInfo *OutputInfo, stopChan chan bool) {
-	var msg string
-	var err error
-	var caseInfo CaseInfo
+// concurrentlyVerifySingleCase verifies a single case in concurrency mode,
+// it will call the cancel function if the case fails and the fail-fast is enabled.
+func concurrentlyVerifySingleCase(
+	ctx context.Context,
+	cancel context.CancelFunc,
+	v *config.VerifyCase,
+	verifyInfo *verifyInfo,
+) (res *output.CaseResult) {
+	res = &output.CaseResult{}
 	defer func() {
-		caseInfo = CaseInfo{
-			msg,
-			err,
+		if res.Err != nil && verifyInfo.failFast {
+			cancel()
 		}
-		outputInfo.writeLock.Lock()
-		outputInfo.casesInfo = append(outputInfo.casesInfo, caseInfo)
-		outputInfo.writeLock.Unlock()
-		if verifyInfo.failFast {
-			if err != nil {
-				stopChan <- true
-			} else {
-				stopChan <- false
-			}
-		}
-		wg.Done()
 	}()
 
 	if v.GetExpected() == "" {
-		msg = fmt.Sprintf("failed to verify %v:", caseName(v))
-		err = fmt.Errorf("the expected data file for %v is not specified", caseName(v))
-		return
+		res.Msg = fmt.Sprintf("failed to verify %v:", caseName(v))
+		res.Err = fmt.Errorf("the expected data file for %v is not specified", caseName(v))
+		return res
 	}
 
 	for current := 0; current <= verifyInfo.retryCount; current++ {
-		if err = verifySingleCase(v.GetExpected(), v.GetActual(), v.Query); err == nil {
-			if current == 0 {
-				msg = fmt.Sprintf("verified %v\n", caseName(v))
+		select {
+		case <-ctx.Done():
+			res.Skip = true
+			return res
+		default:
+			if err := verifySingleCase(v.GetExpected(), v.GetActual(), v.Query); err == nil {
+				if current == 0 {
+					res.Msg = fmt.Sprintf("verified %v\n", caseName(v))
+				} else {
+					res.Msg = fmt.Sprintf("verified %v, retried %d time(s)\n", caseName(v), current)
+				}
+				return res
+			} else if current != verifyInfo.retryCount {
+				time.Sleep(verifyInfo.interval)
 			} else {
-				msg = fmt.Sprintf("verified %v, retried %d time(s)\n", caseName(v), current)
+				res.Msg = fmt.Sprintf("failed to verify %v, retried %d time(s):", caseName(v), current)
+				res.Err = err
 			}
-			return
-		} else if current != verifyInfo.retryCount {
-			time.Sleep(verifyInfo.interval)
-		} else {
-			msg = fmt.Sprintf("failed to verify %v, retried %d time(s):", caseName(v), current)
 		}
 	}
+
+	return res
 }
 
 // verifyCasesConcurrently verifies the cases concurrently.
 func verifyCasesConcurrently(verify *config.Verify, verifyInfo *verifyInfo) error {
-	summary := Summary{}
-	var waitGroup sync.WaitGroup
-	stopChan := make(chan bool)
-	waitGroup.Add(verifyInfo.caseNumber)
-	outputInfo := OutputInfo{}
-	for idx := range verify.Cases {
-		go concurrentlyVerifySingleCase(&verify.Cases[idx], verifyInfo, &waitGroup, &outputInfo, stopChan)
+	res := make([]*output.CaseResult, len(verify.Cases))
+	for i := range res {
+		res[i] = &output.CaseResult{}
 	}
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
 
-	if verifyInfo.failFast {
-		if shouldExit(stopChan, verifyInfo.caseNumber) {
-			outputResult(&outputInfo, &summary)
-			outputSummary(&summary, verifyInfo.caseNumber)
-			return fmt.Errorf("failed to verify one case")
-		}
+	var wg sync.WaitGroup
+	for idx := range verify.Cases {
+		wg.Add(1)
+		go func(i int) {
+			defer wg.Done()
+
+			// Check if the context is canceled before verifying the case.
+			select {
+			case <-ctx.Done():
+				res[i].Skip = true
+				return
+			default:
+				// It's safe to do this, since each goroutine only modifies a single, different, designated slice element.
+				res[i] = concurrentlyVerifySingleCase(ctx, cancel, &verify.Cases[i], verifyInfo)
+			}
+		}(idx)
 	}
-	waitGroup.Wait()
-	outputResult(&outputInfo, &summary)
-	outputSummary(&summary, verifyInfo.caseNumber)
-	if summary.errNum > 0 {
-		return fmt.Errorf("failed to verify %d case(s)", summary.errNum)
+	wg.Wait()
+
+	_, errNum, _ := printer.PrintResult(res)
+	if errNum > 0 {
+		return fmt.Errorf("failed to verify %d case(s)", errNum)
 	}
+
 	return nil
 }
 
 // verifyCasesSerially verifies the cases serially.
-func verifyCasesSerially(verify *config.Verify, verifyInfo *verifyInfo) error {
-	summary := Summary{}
+func verifyCasesSerially(verify *config.Verify, verifyInfo *verifyInfo) (err error) {
+	// A case may be skipped in fail-fast mode, so set it in advance.
+	res := make([]*output.CaseResult, len(verify.Cases))
+	for i := range res {
+		res[i] = &output.CaseResult{
+			Skip: true,
+		}
+	}
+
+	defer func() {
+		_, errNum, _ := printer.PrintResult(res)
+		if errNum > 0 {
+			err = fmt.Errorf("failed to verify %d case(s)", errNum)
+		}
+	}()
+
 	for idx := range verify.Cases {
+		printer.Start()
 		v := &verify.Cases[idx]
-		spinnerLiveText, _ := pterm.DefaultSpinner.WithShowTimer(false).Start()
-		spinnerLiveText.MessageStyle = &pterm.Style{pterm.FgCyan}
-		pterm.Error.Prefix = pterm.Prefix{
-			Text:  "DETAILS",
-			Style: &pterm.ThemeDefault.ErrorPrefixStyle,
-		}
 
 		if v.GetExpected() == "" {
-			errMsg := fmt.Sprintf("failed to verify %v", caseName(v))
-			spinnerLiveText.Warning(errMsg)
-			spinnerLiveText.Fail(fmt.Sprintf("the expected data file for %v is not specified\n", caseName(v)))
-			summary.errNum++
+			res[idx].Skip = false
+			res[idx].Msg = fmt.Sprintf("failed to verify %v", caseName(v))
+			res[idx].Err = fmt.Errorf("the expected data file for %v is not specified", caseName(v))
+
+			printer.Warning(res[idx].Msg)
+			printer.Fail(res[idx].Err.Error())
 			if verifyInfo.failFast {
-				outputSummary(&summary, verifyInfo.caseNumber)
-				return fmt.Errorf("failed to verify one case")
+				return
 			}
 			continue
 		}
 
 		for current := 0; current <= verifyInfo.retryCount; current++ {
-			if err := verifySingleCase(v.GetExpected(), v.GetActual(), v.Query); err == nil {
-				summary.successNum++
+			if e := verifySingleCase(v.GetExpected(), v.GetActual(), v.Query); e == nil {
 				if current == 0 {
-					spinnerLiveText.Success(fmt.Sprintf("verified %v \n", caseName(v)))
+					res[idx].Msg = fmt.Sprintf("verified %v \n", caseName(v))
 				} else {
-					spinnerLiveText.Success(fmt.Sprintf("verified %v, retried %d time(s)\n", caseName(v), current))
+					res[idx].Msg = fmt.Sprintf("verified %v, retried %d time(s)\n", caseName(v), current)
 				}
+				res[idx].Skip = false
+				printer.Success(res[idx].Msg)
 				break
 			} else if current != verifyInfo.retryCount {
 				if current == 0 {
-					spinnerLiveText.UpdateText(fmt.Sprintf("failed to verify %v, will continue retry:", caseName(v)))
-					time.Sleep(time.Second * 2)
+					printer.UpdateText(fmt.Sprintf("failed to verify %v, will continue retry:", caseName(v)))
 				} else {
-					spinnerLiveText.UpdateText(fmt.Sprintf("failed to verify %v, retry [%d/%d]", caseName(v), current, verifyInfo.retryCount))
-					time.Sleep(verifyInfo.interval)
+					printer.UpdateText(fmt.Sprintf("failed to verify %v, retry [%d/%d]", caseName(v), current, verifyInfo.retryCount))
 				}
+				time.Sleep(verifyInfo.interval)
 			} else {
-				summary.errNum++
-				spinnerLiveText.UpdateText(fmt.Sprintf("failed to verify %v, retry [%d/%d]", caseName(v), current, verifyInfo.retryCount))
-				time.Sleep(time.Second)
-				spinnerLiveText.Warning(fmt.Sprintf("failed to verify %v, retried %d time(s):", caseName(v), current))
-				spinnerLiveText.Fail(err)
-				fmt.Println()
+				res[idx].Msg = fmt.Sprintf("failed to verify %v, retried %d time(s):", caseName(v), current)
+				res[idx].Err = e
+				res[idx].Skip = false
+				printer.UpdateText(fmt.Sprintf("failed to verify %v, retry [%d/%d]", caseName(v), current, verifyInfo.retryCount))
+				printer.Warning(res[idx].Msg)
+				printer.Fail(res[idx].Err.Error())
 				if verifyInfo.failFast {
-					outputSummary(&summary, verifyInfo.caseNumber)
-					return fmt.Errorf("failed to verify one case, an error occurred")
+					return
 				}
 			}
 		}
 	}
 
-	outputSummary(&summary, verifyInfo.caseNumber)
-	if summary.errNum > 0 {
-		return fmt.Errorf("failed to verify %d case(s)", summary.errNum)
-	}
 	return nil
 }
 
-// outputSummary outputs a summary of verify result. The summary shows the number of the successful, failed and skipped cases.
-func outputSummary(summary *Summary, total int) {
-	pterm.Info.Prefix = pterm.Prefix{
-		Text:  "SUMMARY",
-		Style: &pterm.ThemeDefault.InfoPrefixStyle,
-	}
-	pterm.Info.WithMessageStyle(&pterm.Style{pterm.FgGreen}).Println(fmt.Sprintf("%d passed", summary.successNum))
-	pterm.Info.Prefix = pterm.Prefix{
-		Text:  "       ",
-		Style: &pterm.ThemeDefault.InfoPrefixStyle,
-	}
-	pterm.Info.WithMessageStyle(&pterm.Style{pterm.FgLightRed}).Println(fmt.Sprintf("%d failed", summary.errNum))
-	pterm.Info.WithMessageStyle(&pterm.Style{pterm.FgYellow}).Println(fmt.Sprintf("%d skipped", total-summary.errNum-summary.successNum))
-	fmt.Println()
-}
-
-// outputResult outputs the result of cases.
-func outputResult(outputInfo *OutputInfo, summary *Summary) {
-	pterm.Error.Prefix = pterm.Prefix{
-		Text:  "DETAILS",
-		Style: &pterm.ThemeDefault.ErrorPrefixStyle,
-	}
-	for _, caseInfo := range outputInfo.casesInfo {
-		if caseInfo.err == nil {
-			summary.successNum++
-			pterm.DefaultSpinner.Success(caseInfo.msg)
-		} else {
-			summary.errNum++
-			pterm.DefaultSpinner.Warning(caseInfo.msg)
-			pterm.DefaultSpinner.Fail(caseInfo.err)
-		}
-	}
-}
-
 func caseName(v *config.VerifyCase) string {
 	if v.Name == "" {
 		if v.Actual != "" {
@@ -287,22 +257,6 @@ func caseName(v *config.VerifyCase) string {
 	return v.Name
 }
 
-func shouldExit(stopChan chan bool, goroutineNum int) bool {
-	count := 0
-	for shouldExit := range stopChan {
-		count++
-
-		if shouldExit {
-			return true
-		}
-
-		if count == goroutineNum {
-			break
-		}
-	}
-	return false
-}
-
 // DoVerifyAccordingConfig reads cases from the config file and verifies them.
 func DoVerifyAccordingConfig() error {
 	if config.GlobalConfig.Error != nil {
@@ -329,9 +283,12 @@ func DoVerifyAccordingConfig() error {
 
 	concurrency := e2eConfig.Verify.Concurrency
 	if concurrency {
+		// enable batch output mode when concurrency is enabled
+		printer = output.NewPrinter(true)
 		return verifyCasesConcurrently(&e2eConfig.Verify, &VerifyInfo)
 	}
 
+	printer = output.NewPrinter(util.BatchMode)
 	return verifyCasesSerially(&e2eConfig.Verify, &VerifyInfo)
 }
 
diff --git a/internal/util/config.go b/internal/util/config.go
index ad082b5..86d99b4 100644
--- a/internal/util/config.go
+++ b/internal/util/config.go
@@ -26,9 +26,10 @@ import (
 )
 
 var (
-	CfgFile string
-	WorkDir string
-	LogDir  string
+	CfgFile   string
+	WorkDir   string
+	LogDir    string
+	BatchMode bool
 )
 
 // ResolveAbs resolves the relative path (relative to CfgFile) to an absolute file path.
diff --git a/pkg/output/printer.go b/pkg/output/printer.go
new file mode 100644
index 0000000..fe23062
--- /dev/null
+++ b/pkg/output/printer.go
@@ -0,0 +1,141 @@
+// Licensed to Apache Software Foundation (ASF) under one or more contributor
+// license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright
+// ownership. Apache Software Foundation (ASF) licenses this file to you under
+// the Apache License, Version 2.0 (the "License"); you may
+// not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package output
+
+import (
+	"fmt"
+
+	"github.com/pterm/pterm"
+)
+
+// CaseResult represents the result of a verification case.
+type CaseResult struct {
+	Msg  string
+	Err  error
+	Skip bool
+}
+
+type Printer interface {
+	Start(...string)
+	Success(string)
+	Warning(string)
+	Fail(string)
+	UpdateText(string)
+	PrintResult([]*CaseResult) (int, int, int)
+}
+
+type printer struct {
+	spinner     *pterm.SpinnerPrinter
+	batchOutput bool
+}
+
+var _ Printer = &printer{}
+
+func NewPrinter(batchOutput bool) Printer {
+	spinner := pterm.DefaultSpinner.WithShowTimer(false)
+	pterm.Error.Prefix = pterm.Prefix{
+		Text:  "DETAILS",
+		Style: &pterm.ThemeDefault.ErrorPrefixStyle,
+	}
+
+	return &printer{
+		spinner:     spinner,
+		batchOutput: batchOutput,
+	}
+}
+
+func (p *printer) Start(msg ...string) {
+	if p.batchOutput {
+		return
+	}
+
+	p.spinner, _ = p.spinner.Start(msg)
+}
+
+func (p *printer) Success(msg string) {
+	if p.batchOutput {
+		return
+	}
+
+	p.spinner.Success(msg)
+}
+
+func (p *printer) Warning(msg string) {
+	if p.batchOutput {
+		return
+	}
+
+	p.spinner.Warning(msg)
+}
+
+func (p *printer) Fail(msg string) {
+	if p.batchOutput {
+		return
+	}
+
+	p.spinner.Fail(msg)
+}
+
+func (p *printer) UpdateText(text string) {
+	if p.batchOutput {
+		return
+	}
+
+	p.spinner.UpdateText(text)
+}
+
+// PrintResult prints the result of verification and the summary.
+// If bathOutput is false, will only print the summary.
+func (p *printer) PrintResult(caseRes []*CaseResult) (passNum, failNum, skipNum int) {
+	// Count the number of passed and failed.
+	// If batchOutput is true, print the result of all cases in a batch.
+	for _, cr := range caseRes {
+		if !cr.Skip {
+			if cr.Err == nil {
+				passNum++
+				if p.batchOutput {
+					p.spinner.Success(cr.Msg)
+				}
+			} else {
+				failNum++
+				if p.batchOutput {
+					p.spinner.Warning(cr.Msg)
+					p.spinner.Fail(cr.Err.Error())
+				}
+			}
+		} else {
+			skipNum++
+		}
+	}
+
+	// Print the summary.
+	pterm.Info.Prefix = pterm.Prefix{
+		Text:  "SUMMARY",
+		Style: &pterm.ThemeDefault.InfoPrefixStyle,
+	}
+	pterm.Info.WithMessageStyle(&pterm.Style{pterm.FgGreen}).Println(fmt.Sprintf("%d passed", passNum))
+	pterm.Info.Prefix = pterm.Prefix{
+		Text:  "       ",
+		Style: &pterm.ThemeDefault.InfoPrefixStyle,
+	}
+	pterm.Info.WithMessageStyle(&pterm.Style{pterm.FgLightRed}).Println(fmt.Sprintf("%d failed", failNum))
+	pterm.Info.WithMessageStyle(&pterm.Style{pterm.FgYellow}).Println(fmt.Sprintf("%d skipped", skipNum))
+	fmt.Println()
+
+	return passNum, failNum, skipNum
+}