You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by ke...@apache.org on 2021/10/01 17:18:52 UTC
[skywalking-infra-e2e] 01/01: Change interval setting to Duration
style, add reasonable default `cleanup.on`
This is an automated email from the ASF dual-hosted git repository.
kezhenxu94 pushed a commit to branch verify/retry-interval
in repository https://gitbox.apache.org/repos/asf/skywalking-infra-e2e.git
commit cf24a47865937e5f5a92ec8b483b2b65b31d9878
Author: kezhenxu94 <ke...@apache.org>
AuthorDate: Sat Oct 2 01:10:10 2021 +0800
Change interval setting to Duration style, add reasonable default `cleanup.on`
---
commands/verify/verify.go | 29 ++++++++++++---
commands/verify/verify_test.go | 71 +++++++++++++++++++++++++++++++++++++
docs/en/setup/Configuration-File.md | 8 +++--
internal/config/e2eConfig.go | 4 +--
internal/config/globalConfig.go | 10 ++++++
5 files changed, 114 insertions(+), 8 deletions(-)
diff --git a/commands/verify/verify.go b/commands/verify/verify.go
index 776510b..996bb73 100644
--- a/commands/verify/verify.go
+++ b/commands/verify/verify.go
@@ -96,9 +96,9 @@ func DoVerifyAccordingConfig() error {
if retryCount <= 0 {
retryCount = 1
}
- retryInterval := e2eConfig.Verify.RetryStrategy.Interval
- if retryInterval < 0 {
- retryInterval = 1000
+ interval, err := parseInterval(e2eConfig.Verify.RetryStrategy.Interval)
+ if err != nil {
+ return err
}
for idx, v := range e2eConfig.Verify.Cases {
@@ -110,7 +110,7 @@ func DoVerifyAccordingConfig() error {
break
} else if current != retryCount {
logger.Log.Warnf("verify case failure, will continue retry, %v", err)
- time.Sleep(time.Duration(retryInterval) * time.Millisecond)
+ time.Sleep(interval)
} else {
return err
}
@@ -119,3 +119,24 @@ func DoVerifyAccordingConfig() error {
return nil
}
+
+func parseInterval(retryInterval interface{}) (time.Duration, error) {
+ var interval time.Duration
+ var err error
+ switch itv := retryInterval.(type) {
+ case int:
+ logger.Log.Warnf(`configuring verify.retry.interval with number is deprecated
+and will be removed in future version, please use Duration style instead, such as 10s, 1m.`)
+ interval = time.Duration(itv) * time.Millisecond
+ case string:
+ if interval, err = time.ParseDuration(itv); err != nil {
+ return 0, err
+ }
+ default:
+ return 0, fmt.Errorf("failed to parse verify.retry.interval: %v", retryInterval)
+ }
+ if interval < 0 {
+ interval = 1 * time.Second
+ }
+ return interval, nil
+}
diff --git a/commands/verify/verify_test.go b/commands/verify/verify_test.go
new file mode 100644
index 0000000..89c1e80
--- /dev/null
+++ b/commands/verify/verify_test.go
@@ -0,0 +1,71 @@
+// 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 verify
+
+import (
+ "testing"
+ "time"
+)
+
+func Test_parseInterval(t *testing.T) {
+ type args struct {
+ retryInterval interface{}
+ }
+ tests := []struct {
+ name string
+ args args
+ want time.Duration
+ wantErr bool
+ }{
+ {
+ name: "Backward compatibility, should parse numeric value",
+ args: args{retryInterval: 1000},
+ want: time.Second,
+ wantErr: false,
+ },
+ {
+ name: "Should parse duration like 10s",
+ args: args{retryInterval: "10s"},
+ want: 10 * time.Second,
+ wantErr: false,
+ },
+ {
+ name: "Should have default value if < 0",
+ args: args{retryInterval: "-10s"},
+ want: 1 * time.Second,
+ wantErr: false,
+ },
+ {
+ name: "Should fail in other cases",
+ args: args{retryInterval: "abcdef"},
+ wantErr: true,
+ },
+ }
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ got, err := parseInterval(tt.args.retryInterval)
+ if (err != nil) != tt.wantErr {
+ t.Errorf("parseInterval() error = %v, wantErr %v", err, tt.wantErr)
+ return
+ }
+ if got != tt.want {
+ t.Errorf("parseInterval() got = %v, want %v", got, tt.want)
+ }
+ })
+ }
+}
diff --git a/docs/en/setup/Configuration-File.md b/docs/en/setup/Configuration-File.md
index 2c5500f..67ae11f 100644
--- a/docs/en/setup/Configuration-File.md
+++ b/docs/en/setup/Configuration-File.md
@@ -108,7 +108,7 @@ After the `Trigger` step is finished, running test cases.
verify:
retry: # verify with retry strategy
count: 10 # max retry count
- interval: 10000 # the interval between two retries, in millisecond.
+ interval: 10s # the duration between two attempts, e.g. 10s, 1m.
cases: # verify test cases
- actual: path/to/actual.yaml # verify by actual file path
expected: path/to/expected.yaml # excepted content file path
@@ -186,7 +186,11 @@ cleanup:
on: always # Clean up strategy
```
-Supports the following strategies:
+If the `on` option under `cleanup` is not set, it will be automatically set to `always` if there is environment
+variable `CI=true`, which is present on many popular CI services, such as GitHub Actions, CircleCI, etc., otherwise it
+will be set to `success`, so the testing environment can be preserved when tests failed in your local machine.
+
+All available strategies:
1. `always`: No matter the execution result is success or failure, cleanup will be performed.
1. `success`: Only when the execution succeeds.
1. `failure`: Only when the execution failed.
diff --git a/internal/config/e2eConfig.go b/internal/config/e2eConfig.go
index db89f61..60084e1 100644
--- a/internal/config/e2eConfig.go
+++ b/internal/config/e2eConfig.go
@@ -90,8 +90,8 @@ type VerifyCase struct {
}
type VerifyRetryStrategy struct {
- Count int `yaml:"count"`
- Interval int `yaml:"interval"`
+ Count int `yaml:"count"`
+ Interval interface{} `yaml:"interval"`
}
// GetActual resolves the absolute file path of the actual data file.
diff --git a/internal/config/globalConfig.go b/internal/config/globalConfig.go
index 7cef38e..21936ae 100644
--- a/internal/config/globalConfig.go
+++ b/internal/config/globalConfig.go
@@ -21,7 +21,9 @@ package config
import (
"fmt"
"io/ioutil"
+ "os"
+ "github.com/apache/skywalking-infra-e2e/internal/constant"
"github.com/apache/skywalking-infra-e2e/internal/logger"
"github.com/apache/skywalking-infra-e2e/internal/util"
@@ -36,6 +38,14 @@ type GlobalE2EConfig struct {
var GlobalConfig GlobalE2EConfig
+func init() {
+ if os.Getenv("CI") == "true" {
+ GlobalConfig.E2EConfig.Cleanup.On = constant.CleanUpAlways
+ } else {
+ GlobalConfig.E2EConfig.Cleanup.On = constant.CleanUpOnSuccess
+ }
+}
+
func ReadGlobalConfigFile() {
if !util.PathExist(util.CfgFile) {
GlobalConfig.Error = fmt.Errorf("e2e config file %s not exist", util.CfgFile)