fix: Fix smart retry in XCUITest simulator test (#873)

* fix: Fix smart retry for XCUITest simulator test --------- Co-authored-by: Alex Plischke <[email protected]>
saucelabs · Jan 17, 2024 · 5a3c78c · 5a3c78c
1 parent c95eb7b
commit 5a3c78c
Show file tree

Hide file tree

Showing 3 changed files with 69 additions and 10 deletions.
diff --git a/internal/cmd/run/xcuitest.go b/internal/cmd/run/xcuitest.go
@@ -174,6 +174,7 @@ func runXcuitestInCloud(p xcuitest.Project, regio region.Region) (int, error) {
 			Async:     gFlags.async,
 			FailFast:  gFlags.failFast,
 			Retrier: &retry.JunitRetrier{
+				VDCReader: &restoClient,
 				RDCReader: &rdcClient,
 			},
 		},

diff --git a/internal/saucecloud/retry/junitretrier.go b/internal/saucecloud/retry/junitretrier.go
@@ -3,6 +3,7 @@ package retry
 import (
 	"context"
 	"fmt"
+	"strings"
 
 	"github.com/rs/zerolog/log"
 	"github.com/saucelabs/saucectl/internal/job"
@@ -39,22 +40,30 @@ func (b *JunitRetrier) retryFailedTests(reader job.Reader, jobOpts chan<- job.St
 
 // setClassesToRetry sets the correct filtering flag when retrying.
 // RDC API does not provide different endpoints (or identical values) for Espresso
-// and XCUITest. Thus, we need set the classes at the correct position depending the
-// framework that is being executed.
+// and XCUITest. Thus, we need set the classes at the correct position depending
+// on the framework that is being executed.
 func setClassesToRetry(opt *job.StartOptions, testcases []junit.TestCase) {
 	lg := log.Info().
 		Str("suite", opt.DisplayName).
 		Str("attempt", fmt.Sprintf("%d of %d", opt.Attempt+1, opt.Retries+1))
 
+	if opt.TestOptions == nil {
+		opt.TestOptions = map[string]interface{}{}
+	}
+
 	if opt.Framework == xcuitest.Kind {
-		opt.TestsToRun = getFailedXCUITests(testcases)
-		lg.Msgf(msg.RetryWithTests, opt.TestsToRun)
+		tests := getFailedXCUITests(testcases)
+
+		// RDC and VDC API filter use different fields for test filtering.
+		if opt.RealDevice {
+			opt.TestsToRun = tests
+		} else {
+			opt.TestOptions["class"] = tests
+		}
+		lg.Msgf(msg.RetryWithTests, tests)
 		return
 	}
 
-	if opt.TestOptions == nil {
-		opt.TestOptions = map[string]interface{}{}
-	}
 	tests := getFailedEspressoTests(testcases)
 	opt.TestOptions["class"] = tests
 	lg.Msgf(msg.RetryWithTests, tests)
@@ -84,16 +93,30 @@ func getFailedXCUITests(testCases []junit.TestCase) []string {
 	classes := map[string]bool{}
 	for _, tc := range testCases {
 		if tc.Error != nil || tc.Failure != nil {
+			className := normalizeXCUITestClassName(tc.ClassName)
 			if tc.Name != "" {
-				classes[fmt.Sprintf("%s/%s", tc.ClassName, tc.Name)] = true
+				classes[fmt.Sprintf("%s/%s", className, tc.Name)] = true
 			} else {
-				classes[tc.ClassName] = true
+				classes[className] = true
 			}
 		}
 	}
 	return maps.Keys(classes)
 }
 
+// normalizeXCUITestClassName normalizes the class name of an XCUITest. The
+// class name within the platform generated JUnit XML file can be dot-separated,
+// but our platform API expects a slash-separated class name. The platform is
+// unfortunately not consistent in this regard and is not in full control of the
+// generated JUnit XML file, hence we reconcile the two here.
+func normalizeXCUITestClassName(name string) string {
+	items := strings.Split(name, ".")
+	if len(items) == 1 {
+		return name
+	}
+	return strings.Join(items, "/")
+}
+
 // getFailedEspressoTests returns a list of failed Espresso tests from the given
 // test cases. The format is "<className>#<testMethodName>", with the test
 // method name being optional.

diff --git a/internal/saucecloud/retry/junitretrier_test.go b/internal/saucecloud/retry/junitretrier_test.go
@@ -185,6 +185,7 @@ func TestAppsRetrier_Retry(t *testing.T) {
 					SmartRetry: job.SmartRetry{
 						FailedOnly: true,
 					},
+					RealDevice: true,
 				},
 				previous: job.Job{
 					ID:    "fake-job-id",
@@ -194,10 +195,12 @@ func TestAppsRetrier_Retry(t *testing.T) {
 			expected: job.StartOptions{
 				Framework:   xcuitest.Kind,
 				DisplayName: "Dummy Test",
-				TestsToRun:  []string{"Demo.Class1/demoTest"},
+				TestOptions: map[string]interface{}{},
+				TestsToRun:  []string{"Demo/Class1/demoTest"},
 				SmartRetry: job.SmartRetry{
 					FailedOnly: true,
 				},
+				RealDevice: true,
 			},
 		},
 		{
@@ -370,3 +373,35 @@ func TestAppsRetrier_Retry(t *testing.T) {
 		})
 	}
 }
+
+func Test_normalizeXCUITestClassName(t *testing.T) {
+	type args struct {
+		name string
+	}
+	tests := []struct {
+		name string
+		args args
+		want string
+	}{
+		{
+			name: "needs normalization",
+			args: args{name: "DemoAppTests.ClassyTest"},
+			want: "DemoAppTests/ClassyTest",
+		},
+		{
+			name: "already normalized",
+			args: args{name: "DemoAppTests/ClassyTest"},
+			want: "DemoAppTests/ClassyTest",
+		},
+		{
+			name: "nothing to normalize",
+			args: args{name: "DemoAppTests"},
+			want: "DemoAppTests",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equalf(t, tt.want, normalizeXCUITestClassName(tt.args.name), "normalizeXCUITestClassName(%v)", tt.args.name)
+		})
+	}
+}