You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2021/03/22 03:31:16 UTC

[GitHub] [skywalking-infra-e2e] kezhenxu94 commented on a change in pull request #15: Refactor setup part

kezhenxu94 commented on a change in pull request #15:
URL: https://github.com/apache/skywalking-infra-e2e/pull/15#discussion_r598402955



##########
File path: commands/run/run.go
##########
@@ -51,8 +54,27 @@ func runAccordingE2E() error {
 	logger.Log.Infof("setup part finished successfully")
 
 	// trigger part
+	err = trigger.DoActionAccordingE2E()
+	if err != nil {
+		return err
+	}
+	logger.Log.Infof("trigger part finished successfully")
 
 	// verify part
+	err = verify.DoVerifyAccordingConfig()
+	if err != nil {
+		return err
+	}
+	logger.Log.Infof("verify part finished successfully")
+
+	// cleanup part
+	defer func() {
+		err = cleanup.DoCleanupAccordingE2E()
+		if err != nil {
+			logger.Log.Errorf("cleanup part error: %s", err)
+		}
+		logger.Log.Infof("cleanup part finished successfully")
+	}()

Review comment:
       Should move this to right after the `setup` part.
   
   Also, please consider adding an option (called `on`) to `cleanup` section to specify when we should clean up the resources, for example, `always`, `on-success`, so that users can see what happened when it failed

##########
File path: internal/config/e2eConfig.go
##########
@@ -28,11 +28,17 @@ type E2EConfig struct {
 }
 
 type Setup struct {
-	Env       string     `yaml:"env"`
-	File      string     `yaml:"file"`
-	Manifests []Manifest `yaml:"manifests"`
-	Runs      []Run      `yaml:"runs"`
-	Timeout   int        `yaml:"timeout"`
+	Env     string `yaml:"env"`
+	File    string `yaml:"file"`
+	Steps   []Step `yaml:"steps"`
+	Timeout int    `yaml:"timeout"`
+}
+
+type Step struct {
+	Type    string `yaml:"type"`

Review comment:
       Can we remove this field and just judge from `Path != ""` / `Command != ""`, and throw an error when `Path == "" && Command == ""`?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org