From edf2829b9ec96351224f30a70b0bd2c475063495 Mon Sep 17 00:00:00 2001 From: Mike Han Date: Thu, 10 Aug 2023 11:33:47 -0600 Subject: [PATCH 01/12] Add more test options and conversion to map[string]interface{} --- internal/saucecloud/xcuitest.go | 5 +---- internal/xcuitest/config.go | 29 +++++++++++++++++++++++++++-- internal/xcuitest/config_test.go | 19 +++++++++++++++++++ 3 files changed, 47 insertions(+), 6 deletions(-) diff --git a/internal/saucecloud/xcuitest.go b/internal/saucecloud/xcuitest.go index 8b09a0858..a1379cff2 100644 --- a/internal/saucecloud/xcuitest.go +++ b/internal/saucecloud/xcuitest.go @@ -220,10 +220,7 @@ func (r *XcuitestRunner) startJob(jobOpts chan<- job.StartOptions, appFileID, te SmartRetry: job.SmartRetry{ FailedOnly: s.SmartRetry.IsRetryFailedOnly(), }, - TestOptions: map[string]interface{}{ - "class": s.TestOptions.Class, - "notClass": s.TestOptions.NotClass, - }, + TestOptions: s.TestOptions.ToMap(), // RDC Specific flags RealDevice: d.isRealDevice, diff --git a/internal/xcuitest/config.go b/internal/xcuitest/config.go index 9adc2a276..0239f6c5b 100644 --- a/internal/xcuitest/config.go +++ b/internal/xcuitest/config.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "os" + "reflect" "strings" "time" @@ -55,8 +56,32 @@ type Xcuitest struct { // TestOptions represents the xcuitest test filter options configuration. type TestOptions struct { - NotClass []string `yaml:"notClass,omitempty" json:"notClass"` - Class []string `yaml:"class,omitempty" json:"class"` + NotClass []string `yaml:"notClass,omitempty" json:"notClass"` + Class []string `yaml:"class,omitempty" json:"class"` + TestLanguage string `yaml:"testLanguage,omitempty" json:"testLanguage"` + TestRegion string `yaml:"testRegion,omitempty" json:"testRegion"` + TestTimeoutsEnabled string `yaml:"testTimeoutsEnabled,omitempty" json:"testTimeoutsEnabled"` + MaximumTestExecutionTimeAllowance int `yaml:"maximumTestExecutionTimeAllowance,omitempty" json:"maximumTestExecutionTimeAllowance"` + DefaultTestExecutionTimeAllowance int `yaml:"defaultTestExecutionTimeAllowance,omitempty" json:"defaultTestExecutionTimeAllowance"` +} + +// ToMap converts the TestOptions to a map where the keys are dervied from json struct tag. +func (t TestOptions) ToMap() map[string]interface{} { + m := make(map[string]interface{}) + v := reflect.ValueOf(t) + tt := v.Type() + + count := v.NumField() + for i := 0; i < count; i++ { + if v.Field(i).CanInterface() { + tag := tt.Field(i).Tag + tname, ok := tag.Lookup("json") + if ok && tname != "-" { + m[tname] = v.Field(i).Interface() + } + } + } + return m } // Suite represents the xcuitest test suite configuration. diff --git a/internal/xcuitest/config_test.go b/internal/xcuitest/config_test.go index a9fec149b..994af5e89 100644 --- a/internal/xcuitest/config_test.go +++ b/internal/xcuitest/config_test.go @@ -15,6 +15,25 @@ import ( "gotest.tools/v3/fs" ) +func TestToMap(t *testing.T) { + opts := TestOptions{ + Class: []string{}, + NotClass: []string{}, + TestLanguage: "", + TestRegion: "", + TestTimeoutsEnabled: "", + MaximumTestExecutionTimeAllowance: 0, + DefaultTestExecutionTimeAllowance: 0, + } + wantLength := 7 + + m := opts.ToMap() + + if len(m) != wantLength { + t.Errorf("Length of converted TestOptions should match original, got (%v) want (%v)", len(m), wantLength) + } +} + func TestValidate(t *testing.T) { dir := fs.NewDir(t, "xcuitest-config", fs.WithFile("test.ipa", "", fs.WithMode(0655)), From 7e5fd27ca3d0384eb4e23e8dae6fbcac17b15771 Mon Sep 17 00:00:00 2001 From: Mike Han Date: Thu, 10 Aug 2023 11:34:14 -0600 Subject: [PATCH 02/12] Add new testOptions to json schema --- api/saucectl.schema.json | 24 ++++++++++++++++++++++ api/v1alpha/framework/xcuitest.schema.json | 21 +++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/api/saucectl.schema.json b/api/saucectl.schema.json index 53b001836..6dc1619e1 100644 --- a/api/saucectl.schema.json +++ b/api/saucectl.schema.json @@ -2176,6 +2176,30 @@ "notClass": { "description": "Run all classes except those specified here.", "type": "array" + }, + "testLanguage": { + "description": "Specifies ISO 639-1 language during testing.", + "type": "string" + }, + "testRegion": { + "description": "Specifies ISO 3166-1 region during testing.", + "type": "string" + }, + "testTimeoutsEnabled": { + "description": "By default there is no timeout, if enabled, then the timeout is 600 seconds. This can be changed by adding the defaultTestExecutionTimeAllowance value.", + "type": "string", + "enum": [ + "Yes", + "No" + ] + }, + "maximumTestExecutionTimeAllowance": { + "description": "The maximum execution time an individual test is given to execute, regardless of the test's preferred allowance.", + "type": "number" + }, + "defaultTestExecutionTimeAllowance": { + "description": "The default execution time an individual test is given to execute if test timeouts are enabled.", + "type": "number" } }, "additionalProperties": false diff --git a/api/v1alpha/framework/xcuitest.schema.json b/api/v1alpha/framework/xcuitest.schema.json index 1b25bf2ce..8d39d1c71 100644 --- a/api/v1alpha/framework/xcuitest.schema.json +++ b/api/v1alpha/framework/xcuitest.schema.json @@ -103,6 +103,27 @@ "notClass": { "description": "Run all classes except those specified here.", "type": "array" + }, + "testLanguage": { + "description": "Specifies ISO 639-1 language during testing.", + "type": "string" + }, + "testRegion": { + "description": "Specifies ISO 3166-1 region during testing.", + "type": "string" + }, + "testTimeoutsEnabled": { + "description": "By default there is no timeout, if enabled, then the timeout is 600 seconds. This can be changed by adding the defaultTestExecutionTimeAllowance value.", + "type": "string", + "enum": ["Yes", "No"] + }, + "maximumTestExecutionTimeAllowance": { + "description": "The maximum execution time an individual test is given to execute, regardless of the test's preferred allowance.", + "type": "number" + }, + "defaultTestExecutionTimeAllowance": { + "description": "The default execution time an individual test is given to execute if test timeouts are enabled.", + "type": "number" } }, "additionalProperties": false From 2de15eefab69d1334926432b8aaf283494a3fc48 Mon Sep 17 00:00:00 2001 From: Mike Han Date: Thu, 10 Aug 2023 11:36:18 -0600 Subject: [PATCH 03/12] note that new props are simulator only --- api/saucectl.schema.json | 8 ++++---- api/v1alpha/framework/xcuitest.schema.json | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/api/saucectl.schema.json b/api/saucectl.schema.json index 6dc1619e1..cf26fc33e 100644 --- a/api/saucectl.schema.json +++ b/api/saucectl.schema.json @@ -2178,7 +2178,7 @@ "type": "array" }, "testLanguage": { - "description": "Specifies ISO 639-1 language during testing.", + "description": "Specifies ISO 639-1 language during testing. Supported on Simulators only.", "type": "string" }, "testRegion": { @@ -2186,7 +2186,7 @@ "type": "string" }, "testTimeoutsEnabled": { - "description": "By default there is no timeout, if enabled, then the timeout is 600 seconds. This can be changed by adding the defaultTestExecutionTimeAllowance value.", + "description": "By default there is no timeout, if enabled, then the timeout is 600 seconds. This can be changed by adding the defaultTestExecutionTimeAllowance value. Supported on Simulators only.", "type": "string", "enum": [ "Yes", @@ -2194,11 +2194,11 @@ ] }, "maximumTestExecutionTimeAllowance": { - "description": "The maximum execution time an individual test is given to execute, regardless of the test's preferred allowance.", + "description": "The maximum execution time an individual test is given to execute, regardless of the test's preferred allowance. Supported on Simulators only.", "type": "number" }, "defaultTestExecutionTimeAllowance": { - "description": "The default execution time an individual test is given to execute if test timeouts are enabled.", + "description": "The default execution time an individual test is given to execute if test timeouts are enabled. Supported on Simulators only.", "type": "number" } }, diff --git a/api/v1alpha/framework/xcuitest.schema.json b/api/v1alpha/framework/xcuitest.schema.json index 8d39d1c71..8d8d5753d 100644 --- a/api/v1alpha/framework/xcuitest.schema.json +++ b/api/v1alpha/framework/xcuitest.schema.json @@ -105,7 +105,7 @@ "type": "array" }, "testLanguage": { - "description": "Specifies ISO 639-1 language during testing.", + "description": "Specifies ISO 639-1 language during testing. Supported on Simulators only.", "type": "string" }, "testRegion": { @@ -113,16 +113,16 @@ "type": "string" }, "testTimeoutsEnabled": { - "description": "By default there is no timeout, if enabled, then the timeout is 600 seconds. This can be changed by adding the defaultTestExecutionTimeAllowance value.", + "description": "By default there is no timeout, if enabled, then the timeout is 600 seconds. This can be changed by adding the defaultTestExecutionTimeAllowance value. Supported on Simulators only.", "type": "string", "enum": ["Yes", "No"] }, "maximumTestExecutionTimeAllowance": { - "description": "The maximum execution time an individual test is given to execute, regardless of the test's preferred allowance.", + "description": "The maximum execution time an individual test is given to execute, regardless of the test's preferred allowance. Supported on Simulators only.", "type": "number" }, "defaultTestExecutionTimeAllowance": { - "description": "The default execution time an individual test is given to execute if test timeouts are enabled.", + "description": "The default execution time an individual test is given to execute if test timeouts are enabled. Supported on Simulators only.", "type": "number" } }, From fed6bfb1b0cb9c1cce5818d3e6b363fddcbd24a6 Mon Sep 17 00:00:00 2001 From: Mike Han Date: Thu, 10 Aug 2023 11:36:49 -0600 Subject: [PATCH 04/12] lint --- internal/xcuitest/config_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/xcuitest/config_test.go b/internal/xcuitest/config_test.go index 994af5e89..6fa056897 100644 --- a/internal/xcuitest/config_test.go +++ b/internal/xcuitest/config_test.go @@ -17,11 +17,11 @@ import ( func TestToMap(t *testing.T) { opts := TestOptions{ - Class: []string{}, - NotClass: []string{}, - TestLanguage: "", - TestRegion: "", - TestTimeoutsEnabled: "", + Class: []string{}, + NotClass: []string{}, + TestLanguage: "", + TestRegion: "", + TestTimeoutsEnabled: "", MaximumTestExecutionTimeAllowance: 0, DefaultTestExecutionTimeAllowance: 0, } From e619a2e88248cf72dde87db262bc0cfd08ac8729 Mon Sep 17 00:00:00 2001 From: Mike Han Date: Thu, 10 Aug 2023 11:38:37 -0600 Subject: [PATCH 05/12] typo --- internal/xcuitest/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/xcuitest/config.go b/internal/xcuitest/config.go index 0239f6c5b..13ad6eccf 100644 --- a/internal/xcuitest/config.go +++ b/internal/xcuitest/config.go @@ -65,7 +65,7 @@ type TestOptions struct { DefaultTestExecutionTimeAllowance int `yaml:"defaultTestExecutionTimeAllowance,omitempty" json:"defaultTestExecutionTimeAllowance"` } -// ToMap converts the TestOptions to a map where the keys are dervied from json struct tag. +// ToMap converts the TestOptions to a map where the keys are dervied from json struct tags. func (t TestOptions) ToMap() map[string]interface{} { m := make(map[string]interface{}) v := reflect.ValueOf(t) From aec86da737372736611cc948a804f9301b662d00 Mon Sep 17 00:00:00 2001 From: Mike Han Date: Thu, 10 Aug 2023 12:43:52 -0600 Subject: [PATCH 06/12] Mention time units --- api/saucectl.schema.json | 4 ++-- api/v1alpha/framework/xcuitest.schema.json | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/api/saucectl.schema.json b/api/saucectl.schema.json index cf26fc33e..62ea528b6 100644 --- a/api/saucectl.schema.json +++ b/api/saucectl.schema.json @@ -2194,11 +2194,11 @@ ] }, "maximumTestExecutionTimeAllowance": { - "description": "The maximum execution time an individual test is given to execute, regardless of the test's preferred allowance. Supported on Simulators only.", + "description": "The maximum execution time, in seconds, an individual test is given to execute, regardless of the test's preferred allowance. Supported on Simulators only.", "type": "number" }, "defaultTestExecutionTimeAllowance": { - "description": "The default execution time an individual test is given to execute if test timeouts are enabled. Supported on Simulators only.", + "description": "The default execution time, in seconds, an individual test is given to execute if test timeouts are enabled. Supported on Simulators only.", "type": "number" } }, diff --git a/api/v1alpha/framework/xcuitest.schema.json b/api/v1alpha/framework/xcuitest.schema.json index 8d8d5753d..be90045bf 100644 --- a/api/v1alpha/framework/xcuitest.schema.json +++ b/api/v1alpha/framework/xcuitest.schema.json @@ -118,11 +118,11 @@ "enum": ["Yes", "No"] }, "maximumTestExecutionTimeAllowance": { - "description": "The maximum execution time an individual test is given to execute, regardless of the test's preferred allowance. Supported on Simulators only.", + "description": "The maximum execution time, in seconds, an individual test is given to execute, regardless of the test's preferred allowance. Supported on Simulators only.", "type": "number" }, "defaultTestExecutionTimeAllowance": { - "description": "The default execution time an individual test is given to execute if test timeouts are enabled. Supported on Simulators only.", + "description": "The default execution time, in seconds, an individual test is given to execute if test timeouts are enabled. Supported on Simulators only.", "type": "number" } }, From c846857c5edf40213746ee5a8383cb67a26c8020 Mon Sep 17 00:00:00 2001 From: Mike Han <56001373+mhan83@users.noreply.github.com> Date: Thu, 10 Aug 2023 12:44:23 -0600 Subject: [PATCH 07/12] Update internal/xcuitest/config.go Co-authored-by: Alex Plischke --- internal/xcuitest/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/xcuitest/config.go b/internal/xcuitest/config.go index 13ad6eccf..a36703cc0 100644 --- a/internal/xcuitest/config.go +++ b/internal/xcuitest/config.go @@ -65,7 +65,7 @@ type TestOptions struct { DefaultTestExecutionTimeAllowance int `yaml:"defaultTestExecutionTimeAllowance,omitempty" json:"defaultTestExecutionTimeAllowance"` } -// ToMap converts the TestOptions to a map where the keys are dervied from json struct tags. +// ToMap converts the TestOptions to a map where the keys are derived from json struct tags. func (t TestOptions) ToMap() map[string]interface{} { m := make(map[string]interface{}) v := reflect.ValueOf(t) From 28bca2482d0355d20279953827b4d706497ad97d Mon Sep 17 00:00:00 2001 From: Mike Han Date: Thu, 10 Aug 2023 14:18:38 -0600 Subject: [PATCH 08/12] Cast ints to strings --- internal/xcuitest/config.go | 14 +++++++++++++- internal/xcuitest/config_test.go | 12 +++++++++++- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/internal/xcuitest/config.go b/internal/xcuitest/config.go index a36703cc0..855ff92d4 100644 --- a/internal/xcuitest/config.go +++ b/internal/xcuitest/config.go @@ -77,7 +77,19 @@ func (t TestOptions) ToMap() map[string]interface{} { tag := tt.Field(i).Tag tname, ok := tag.Lookup("json") if ok && tname != "-" { - m[tname] = v.Field(i).Interface() + fv := v.Field(i).Interface() + ft := v.Field(i).Type() + switch ft.Kind() { + case reflect.Int: + // Conventionally, when TestOptions are converted to match chef expectations, properties with value "" will be ignored. + if fv.(int) == 0 { + m[tname] = "" + } else { + m[tname] = fmt.Sprintf("%v", fv) + } + default: + m[tname] = fv + } } } } diff --git a/internal/xcuitest/config_test.go b/internal/xcuitest/config_test.go index 6fa056897..f0efbf7d4 100644 --- a/internal/xcuitest/config_test.go +++ b/internal/xcuitest/config_test.go @@ -22,7 +22,7 @@ func TestToMap(t *testing.T) { TestLanguage: "", TestRegion: "", TestTimeoutsEnabled: "", - MaximumTestExecutionTimeAllowance: 0, + MaximumTestExecutionTimeAllowance: 20, DefaultTestExecutionTimeAllowance: 0, } wantLength := 7 @@ -32,6 +32,16 @@ func TestToMap(t *testing.T) { if len(m) != wantLength { t.Errorf("Length of converted TestOptions should match original, got (%v) want (%v)", len(m), wantLength) } + + v := reflect.ValueOf(m["maximumTestExecutionTimeAllowance"]) + vtype := v.Type() + if vtype.Kind() != reflect.String { + t.Errorf("ints should be converted to strings when mapping, got (%v) want (%v)", vtype, reflect.String) + } + + if m["defaultTestExecutionTimeAllowance"] != "" { + t.Errorf("0 values should be cast to empty strings, got (%v)", m["defaultTestExecutionTimeAllowance"]) + } } func TestValidate(t *testing.T) { From a273416f35ea17016ab744c1553c127f9a246aca Mon Sep 17 00:00:00 2001 From: Mike Han Date: Thu, 10 Aug 2023 14:24:04 -0600 Subject: [PATCH 09/12] cleanup --- internal/xcuitest/config_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/xcuitest/config_test.go b/internal/xcuitest/config_test.go index f0efbf7d4..fd0bfeaf4 100644 --- a/internal/xcuitest/config_test.go +++ b/internal/xcuitest/config_test.go @@ -39,8 +39,8 @@ func TestToMap(t *testing.T) { t.Errorf("ints should be converted to strings when mapping, got (%v) want (%v)", vtype, reflect.String) } - if m["defaultTestExecutionTimeAllowance"] != "" { - t.Errorf("0 values should be cast to empty strings, got (%v)", m["defaultTestExecutionTimeAllowance"]) + if v := m["defaultTestExecutionTimeAllowance"]; v != "" { + t.Errorf("0 values should be cast to empty strings, got (%v)", v) } } From 6621f8492f11c5f2c3dca6a96160019adb4ae2fd Mon Sep 17 00:00:00 2001 From: Mike Han Date: Thu, 10 Aug 2023 16:25:25 -0600 Subject: [PATCH 10/12] More context around why test options are being cast --- internal/xcuitest/config.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/xcuitest/config.go b/internal/xcuitest/config.go index 855ff92d4..a5da58742 100644 --- a/internal/xcuitest/config.go +++ b/internal/xcuitest/config.go @@ -80,8 +80,9 @@ func (t TestOptions) ToMap() map[string]interface{} { fv := v.Field(i).Interface() ft := v.Field(i).Type() switch ft.Kind() { + // Convert int to string to match chef expectation that all test option values are strings case reflect.Int: - // Conventionally, when TestOptions are converted to match chef expectations, properties with value "" will be ignored. + // Conventionally, when test options with value "" will be ignored. if fv.(int) == 0 { m[tname] = "" } else { From 7d0b8a76f3ad1d9faf3882fcfc883c01539ff5c0 Mon Sep 17 00:00:00 2001 From: Mike Han Date: Thu, 10 Aug 2023 16:34:50 -0600 Subject: [PATCH 11/12] typo --- internal/xcuitest/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/xcuitest/config.go b/internal/xcuitest/config.go index a5da58742..2aeaf43bf 100644 --- a/internal/xcuitest/config.go +++ b/internal/xcuitest/config.go @@ -82,7 +82,7 @@ func (t TestOptions) ToMap() map[string]interface{} { switch ft.Kind() { // Convert int to string to match chef expectation that all test option values are strings case reflect.Int: - // Conventionally, when test options with value "" will be ignored. + // Conventionally, test options with value "" will be ignored. if fv.(int) == 0 { m[tname] = "" } else { From fa673fa45889eb2346180fbf51818991fe1e0bf5 Mon Sep 17 00:00:00 2001 From: Mike Han Date: Thu, 10 Aug 2023 16:43:14 -0600 Subject: [PATCH 12/12] scope the test function --- internal/xcuitest/config_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/xcuitest/config_test.go b/internal/xcuitest/config_test.go index fd0bfeaf4..38c858058 100644 --- a/internal/xcuitest/config_test.go +++ b/internal/xcuitest/config_test.go @@ -15,7 +15,7 @@ import ( "gotest.tools/v3/fs" ) -func TestToMap(t *testing.T) { +func TestTestOptions_ToMap(t *testing.T) { opts := TestOptions{ Class: []string{}, NotClass: []string{},