[GitHub] [skywalking-infra-e2e] kezhenxu94 commented on a change in pull request #53: Support reuse `verify.cases`

[GitBox <gi...@apache.org>]

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



##########
File path: docs/en/setup/Configuration-File.md
##########
@@ -177,6 +179,20 @@ In order to make the program easier for users to read and use, some code convers
 |-------|------------|-------|------|
 |b64enc|Base64 encode|{{ b64enc "Foo" }}|Zm9v|
 
+### Include cases

Review comment:
       ```suggestion
   ### Reuse cases
   ```

##########
File path: docs/en/setup/Configuration-File.md
##########
@@ -177,6 +179,20 @@ In order to make the program easier for users to read and use, some code convers
 |-------|------------|-------|------|
 |b64enc|Base64 encode|{{ b64enc "Foo" }}|Zm9v|
 
+### Include cases
+
+Could be including multiple cases into one single E2E verify, It's help for reusing the same verify cases.
+
+Here is the reusing verify cases, and using `include` configuration item to include this into E2E config.

Review comment:
       ```suggestion
   Here is the reused verify cases, and using `include` configuration item to include this into E2E config.
   ```

##########
File path: commands/verify/verify.go
##########
@@ -101,7 +101,29 @@ func DoVerifyAccordingConfig() error {
 		return err
 	}
 
-	for idx, v := range e2eConfig.Verify.Cases {
+	if err := verifyCases(retryCount, interval, e2eConfig.Verify.Cases); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+func verifyCases(retryCount int, interval time.Duration, cases []config.VerifyCase) error {
+	for idx, v := range cases {
+		if len(v.Include) > 0 {
+			for _, include := range v.Include {
+				reusingCases, err := config.ReadReusingCases(include)

Review comment:
       I'd rather parsing the `include` when parsing the global config (`ReadGlobalConfigFile`), and convert the `include` to the standard `verify.cases`

##########
File path: docs/en/setup/Configuration-File.md
##########
@@ -114,6 +114,8 @@ verify:
       expected: path/to/expected.yaml   # excepted content file path
     - query: echo 'foo'                 # verify by command execute output
       expected: path/to/expected.yaml   # excepted content file path
+    - include:      # including cases
+        - path/to/cases.yaml            # cases file path

Review comment:
       What I proposed is
   
   ```yaml
   - include: path/to/cases1.yaml
   - include: path/to/cases2.yaml
   ```
   
   The reasons is that we have changes to add more options to the `include` item without breaking compatibility.
   
   
   ```yaml
   - include: path/to/cases1.yaml
     option1: xxx
     option2: yyy
   - include: path/to/cases2.yaml
     option1: xxx
     option2: yyy
   ```
   

##########
File path: docs/en/setup/Configuration-File.md
##########
@@ -177,6 +179,20 @@ In order to make the program easier for users to read and use, some code convers
 |-------|------------|-------|------|
 |b64enc|Base64 encode|{{ b64enc "Foo" }}|Zm9v|
 
+### Include cases
+
+Could be including multiple cases into one single E2E verify, It's help for reusing the same verify cases.

Review comment:
       ```suggestion
   You could include multiple cases into one single E2E verify, It's helpful for reusing the same verify cases.
   ```




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

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