[iOS SDK] Improved e2e tests stability (#2337)

spetrescu84 · web-flow · commit 394bcd64b506 · 2024-10-18T08:45:24.000+01:00
* Improved code retrieving to make sure code is from correct email
Increased retries to 5
Disable retry from command for e2e tests

* Removed spacing

* Added new lines in the script

* Indentation

* no message

* Change to single quote

* Trying non-boolean value

* Changed parameter to retry_tests as - is not allowed

* Temporary disable condition

* Using different emails for ios and macos

* Added back the condition

* PR comments

* PR Comments

* Using numberOfRetries instead of hardcoded

* PR comments

* Remove condition

* PR comments

* PR comments

* Added validation back

* Added CIAM team as codeowners to azure_pipelines

* Changed CodeOwners

* Forcing refresh

* Revert spacing
diff --git a/CODEOWNERS b/CODEOWNERS
@@ -2,9 +2,10 @@
 # Unless a later match takes precedence, these users will be requested
 # for review whenever someone opens a pull request.
 *       @AzureAD/AppleIdentityTeam
-# @AzureAD/AppleIdentityTeam and @AzureAD/MSAL-ObjC-CIAM will be the co-owners of MSAL.project and CHANGELOG.md files
+# @AzureAD/AppleIdentityTeam and @AzureAD/MSAL-ObjC-CIAM will be the co-owners of MSAL.project, CHANGELOG.md and all files under azure_pipelines
 /MSAL/MSAL.xcodeproj/project.pbxproj @AzureAD/AppleIdentityTeam @AzureAD/MSAL-ObjC-CIAM
 CHANGELOG.md @AzureAD/AppleIdentityTeam @AzureAD/MSAL-ObjC-CIAM
+/azure_pipelines/ @AzureAD/AppleIdentityTeam @AzureAD/MSAL-ObjC-CIAM
 # @AzureAD/MSAL-ObjC-CIAM owns any files in the */native_auth
 # directories, subdirectories and other files related to native auth.
 /MSAL/module.modulemap @AzureAD/MSAL-ObjC-CIAM
diff --git a/MSAL/test/integration/native_auth/end_to_end/MSALNativeAuthEndToEndBaseTestCase.swift b/MSAL/test/integration/native_auth/end_to_end/MSALNativeAuthEndToEndBaseTestCase.swift
@@ -37,7 +37,11 @@ class MSALNativeAuthEndToEndBaseTestCase: XCTestCase {
         static let signInEmailPasswordMFAUsernameKey = "sign_in_email_password_mfa_username"
         static let signInEmailPasswordMFANoDefaultAuthMethodUsernameKey = "sign_in_email_password_mfa_no_default_username"
         static let signInEmailCodeUsernameKey = "sign_in_email_code_username"
+        #if !os(macOS)
         static let resetPasswordUsernameKey = "reset_password_username"
+        #else
+        static let resetPasswordUsernameKey = "reset_password_username_macos"
+        #endif
     }
     
     let correlationId = UUID()
diff --git a/MSAL/test/integration/native_auth/end_to_end/otp_code_retriever/MSALNativeAuthEmailCodeRetriever.swift b/MSAL/test/integration/native_auth/end_to_end/otp_code_retriever/MSALNativeAuthEmailCodeRetriever.swift
@@ -29,6 +29,14 @@ class MSALNativeAuthEmailCodeRetriever: XCTestCase {
     private let baseURLString = "https://www.1secmail.com/api/v1/?action="
     private let secondsToWait = 4.0
     private let numberOfRetry = 3
+    private let dateFormatter: DateFormatter = {
+        let dateFormatter = DateFormatter()
+        dateFormatter.dateFormat = "yyyy-MM-dd HH:mm:ss"
+        dateFormatter.locale = Locale(identifier: "en_US_POSIX")
+        dateFormatter.timeZone = TimeZone(secondsFromGMT: 0) //We ignore the timezone
+        return dateFormatter
+    }()
+    private let maximumSecondsSinceEmailReceive = 5.0
 
     func retrieveEmailOTPCode(email: String) async -> String? {
         let comps = email.components(separatedBy: "@")
@@ -76,15 +84,25 @@ class MSALNativeAuthEmailCodeRetriever: XCTestCase {
             }
             if dataDictionary.count > 0 {
                 dataDictionary.sort(by: {($0["id"] as? Int ?? 0) > ($1["id"] as? Int ?? 0)})
-                return dataDictionary.first?["id"] as? Int
-            } else {
-                // log only for the final retry
-                if (retryCounter == 1) {
-                    print("Unexpected behaviour: no email received for the following local: \(local)")
+                if let emailDateString = dataDictionary.first?["date"] as? String,
+                   let emailDate = dateFormatter.date(from: emailDateString) {
+                    let currentDate = Date()
+                    // Email should be newer than 5 seconds otherwise it could be from previous test
+                    // This retry will help with the delay in receiving the emails
+                    if currentDate.timeIntervalSince1970 - emailDate.timeIntervalSince1970  < maximumSecondsSinceEmailReceive {
+                        print ("Email is for current test, last receive date: \(emailDate) current date: \(currentDate)")
+                        return dataDictionary.first?["id"] as? Int
+                    } else {
+                        print ("Email is from previous tests, last receive date: \(emailDate) current date: \(currentDate)")
+                    }
                 }
-                // no emails found, retry
-                return await retrieveLastMessage(local: local, domain: domain, retryCounter: retryCounter - 1)
             }
+            // log only for the final retry
+            if (retryCounter == 1) {
+                print("Unexpected behaviour: no email received for the following local: \(local)")
+            }
+            // no emails found, retry
+            return await retrieveLastMessage(local: local, domain: domain, retryCounter: retryCounter - 1)
         } catch {
             print(error)
             return nil
diff --git a/MSAL/test/integration/native_auth/end_to_end/reset_password/MSALNativeAuthResetPasswordEndToEndTests.swift b/MSAL/test/integration/native_auth/end_to_end/reset_password/MSALNativeAuthResetPasswordEndToEndTests.swift
@@ -24,9 +24,12 @@
 
 import Foundation
 import XCTest
+import MSAL
 
 final class MSALNativeAuthResetPasswordEndToEndTests: MSALNativeAuthEndToEndBaseTestCase {
     // Hero Scenario 3.1.1. SSPR – without automatic sign in
+    private let codeRetryCount = 3
+
     func test_resetPassword_withoutAutomaticSignIn_succeeds() async throws {
         guard let sut = initialisePublicClientApplication(),
               let username = retrieveUsernameForResetPassword()
@@ -52,31 +55,16 @@ final class MSALNativeAuthResetPasswordEndToEndTests: MSALNativeAuthEndToEndBase
         XCTAssertNotNil(resetPasswordStartDelegate.codeLength)
 
         // Now submit the code...
-
-        let passwordRequiredExp = expectation(description: "password required")
-        let resetPasswordVerifyDelegate = ResetPasswordVerifyCodeDelegateSpy(expectation: passwordRequiredExp)
-        
-        guard let code = await retrieveCodeFor(email: username) else {
-            XCTFail("OTP code not retrieved from email")
-            return
-        }
-
-        resetPasswordStartDelegate.newState?.submitCode(code: code, delegate: resetPasswordVerifyDelegate)
-
-        await fulfillment(of: [passwordRequiredExp])
-        XCTAssertTrue(resetPasswordVerifyDelegate.onPasswordRequiredCalled)
-        
-        guard resetPasswordVerifyDelegate.onPasswordRequiredCalled else {
-            XCTFail("onPasswordRequired not called")
-            return
-        }
+        let newPasswordRequiredState = await retrieveAndSubmitCode(resetPasswordStartDelegate: resetPasswordStartDelegate,
+                   username: username,
+                   retries: codeRetryCount)
 
         // Now submit the password...
         let resetPasswordCompletedExp = expectation(description: "reset password completed")
         let resetPasswordRequiredDelegate = ResetPasswordRequiredDelegateSpy(expectation: resetPasswordCompletedExp)
 
         let uniquePassword = generateRandomPassword()
-        resetPasswordVerifyDelegate.newPasswordRequiredState?.submitPassword(password: uniquePassword, delegate: resetPasswordRequiredDelegate)
+        newPasswordRequiredState?.submitPassword(password: uniquePassword, delegate: resetPasswordRequiredDelegate)
 
         await fulfillment(of: [resetPasswordCompletedExp])
         XCTAssertTrue(resetPasswordRequiredDelegate.onResetPasswordCompletedCalled)
@@ -108,31 +96,16 @@ final class MSALNativeAuthResetPasswordEndToEndTests: MSALNativeAuthEndToEndBase
         XCTAssertNotNil(resetPasswordStartDelegate.codeLength)
 
         // Now submit the code...
-
-        let passwordRequiredExp = expectation(description: "password required")
-        let resetPasswordVerifyDelegate = ResetPasswordVerifyCodeDelegateSpy(expectation: passwordRequiredExp)
-        
-        guard let code = await retrieveCodeFor(email: username) else {
-            XCTFail("OTP code not retrieved from email")
-            return
-        }
-
-        resetPasswordStartDelegate.newState?.submitCode(code: code, delegate: resetPasswordVerifyDelegate)
-
-        await fulfillment(of: [passwordRequiredExp])
-        XCTAssertTrue(resetPasswordVerifyDelegate.onPasswordRequiredCalled)
-        
-        guard resetPasswordVerifyDelegate.onPasswordRequiredCalled else {
-            XCTFail("onPasswordRequired not called")
-            return
-        }
+        let newPasswordRequiredState = await retrieveAndSubmitCode(resetPasswordStartDelegate: resetPasswordStartDelegate,
+                   username: username,
+                   retries: codeRetryCount)
 
         // Now submit the password...
         let resetPasswordCompletedExp = expectation(description: "reset password completed")
         let resetPasswordRequiredDelegate = ResetPasswordRequiredDelegateSpy(expectation: resetPasswordCompletedExp)
 
         let uniquePassword = generateRandomPassword()
-        resetPasswordVerifyDelegate.newPasswordRequiredState?.submitPassword(password: uniquePassword, delegate: resetPasswordRequiredDelegate)
+        newPasswordRequiredState?.submitPassword(password: uniquePassword, delegate: resetPasswordRequiredDelegate)
 
         await fulfillment(of: [resetPasswordCompletedExp])
         XCTAssertTrue(resetPasswordRequiredDelegate.onResetPasswordCompletedCalled)
@@ -155,4 +128,27 @@ final class MSALNativeAuthResetPasswordEndToEndTests: MSALNativeAuthEndToEndBase
         XCTAssertNotNil(signInAfterResetPasswordDelegate.result?.idToken)
         XCTAssertNotNil(signInAfterResetPasswordDelegate.result?.account.accountClaims)
     }
+
+    // This method tries to fetch a code from 1secmail API and submit it
+    private func retrieveAndSubmitCode(resetPasswordStartDelegate: ResetPasswordStartDelegateSpy, username: String, retries: Int) async -> ResetPasswordRequiredState? {
+        let passwordRequiredExp = expectation(description: "password required")
+        let resetPasswordVerifyDelegate = ResetPasswordVerifyCodeDelegateSpy(expectation: passwordRequiredExp)
+
+        guard let code = await retrieveCodeFor(email: username) else {
+            XCTFail("OTP code not retrieved from email")
+            return nil
+        }
+
+        resetPasswordStartDelegate.newState?.submitCode(code: code, delegate: resetPasswordVerifyDelegate)
+
+        await fulfillment(of: [passwordRequiredExp])
+        if resetPasswordVerifyDelegate.onResetPasswordVerifyCodeErrorCalled && resetPasswordVerifyDelegate.error?.isInvalidCode == true && retries > 0 {
+            return await retrieveAndSubmitCode(resetPasswordStartDelegate: resetPasswordStartDelegate, username: username, retries: retries - 1)
+        }
+        guard resetPasswordVerifyDelegate.onPasswordRequiredCalled else {
+            XCTFail("onPasswordRequired not called")
+            return nil
+        }
+        return resetPasswordVerifyDelegate.newPasswordRequiredState
+    }
 }
diff --git a/MSAL/test/testplan/MSAL Mac Native Auth E2E Tests.xctestplan b/MSAL/test/testplan/MSAL Mac Native Auth E2E Tests.xctestplan
@@ -9,6 +9,7 @@
     }
   ],
   "defaultOptions" : {
+    "maximumTestRepetitions" : 5,
     "testRepetitionMode" : "retryOnFailure"
   },
   "testTargets" : [
diff --git a/MSAL/test/testplan/MSAL iOS Native Auth E2E Tests.xctestplan b/MSAL/test/testplan/MSAL iOS Native Auth E2E Tests.xctestplan
@@ -9,6 +9,7 @@
     }
   ],
   "defaultOptions" : {
+    "maximumTestRepetitions" : 5,
     "testRepetitionMode" : "retryOnFailure"
   },
   "testTargets" : [
diff --git a/azure_pipelines/automation.yml b/azure_pipelines/automation.yml
@@ -93,6 +93,7 @@ jobs:
       full_path: 'build/Build/Products/MSAL iOS Native Auth E2E Tests_MSAL iOS Native Auth E2E Tests_iphonesimulator17.5-x86_64.xctestrun'
       destination: 'platform=iOS Simulator,name=iPhone 15,OS=17.5'
       sdk: 'iphonesimulator'
+      retry_tests: false
 
 - job: e2e_test_native_auth_mac
   displayName: 'Run MSAL E2E tests for macOS native auth'
@@ -108,6 +109,7 @@ jobs:
       full_path: 'build/Build/Products/MSAL Mac Native Auth E2E Tests_MSAL Mac Native Auth E2E Tests_macosx14.5-x86_64.xctestrun'
       destination: 'platform=macOS'
       sdk: 'macosx'
+      retry_tests: false
 
 - job: cocoapods_lib_lint
   displayName: Run Cocoapods lib lint
diff --git a/azure_pipelines/pr-validation.yml b/azure_pipelines/pr-validation.yml
@@ -168,6 +168,7 @@ jobs:
       full_path: 'build/Build/Products/MSAL iOS Native Auth E2E Tests_MSAL iOS Native Auth E2E Tests_iphonesimulator17.5-x86_64.xctestrun'
       destination: 'platform=iOS Simulator,name=iPhone 15,OS=17.5'
       sdk: 'iphonesimulator'
+      retry_tests: false
 
 - job: e2e_test_native_auth_mac
   displayName: 'Run MSAL E2E tests for macOS native auth'
@@ -187,6 +188,7 @@ jobs:
       full_path: 'build/Build/Products/MSAL Mac Native Auth E2E Tests_MSAL Mac Native Auth E2E Tests_macosx14.5-x86_64.xctestrun'
       destination: 'platform=macOS'
       sdk: 'macosx'
+      retry_tests: false
 
 - job: 'Validate_SPM_Integration'
   displayName: Validate SPM Integration
diff --git a/azure_pipelines/templates/tests-with-conf-file.yml b/azure_pipelines/templates/tests-with-conf-file.yml
@@ -3,6 +3,7 @@ parameters:
   full_path: 'build/Build/Products/MSAL Test Automation (iOS)_iphonesimulator17.5-x86_64.xctestrun'
   destination: 'platform=iOS Simulator,name=iPhone 15,OS=17.5'
   sdk: 'iphonesimulator'
+  retry_tests: true
 
 steps:
   - checkout: self
@@ -58,12 +59,23 @@ steps:
       targetType: 'inline'
       script: |
         ls build/Build/Products/
-        xcodebuild test-without-building \
-            -xctestrun '${{ parameters.full_path }}' \
-            -destination '${{ parameters.destination }}' \
-            -retry-tests-on-failure \
-            -parallel-testing-enabled NO \
-            -resultBundlePath '$(Agent.BuildDirectory)/s/test_output/report.xcresult'      
+        #Use retry-tests-on-failure only if tests don't already use "Maximum Test Repetitions"
+        if ['${{ parameters.retry_tests }}' == true]; then
+            xcodebuild test-without-building \
+                -xctestrun '${{ parameters.full_path }}' \
+                -destination '${{ parameters.destination }}' \
+                -retry-tests-on-failure \
+                -parallel-testing-enabled NO \
+                -resultBundlePath '$(Agent.BuildDirectory)/s/test_output/report.xcresult'
+
+        else
+            xcodebuild test-without-building \
+                -xctestrun '${{ parameters.full_path }}' \
+                -destination '${{ parameters.destination }}' \
+                -parallel-testing-enabled NO \
+                -resultBundlePath '$(Agent.BuildDirectory)/s/test_output/report.xcresult'
+
+        fi 
 
   # https://learn.microsoft.com/en-us/azure/devops/pipelines/artifacts/pipeline-artifacts?view=azure-devops&tabs=yaml#q-can-i-delete-pipeline-artifacts-when-re-running-failed-jobs
   - task: PublishPipelineArtifact@1

Original file line number	Diff line number	Diff line change
`@@ -9,6 +9,7 @@`
`9`	`9`	`}`
`10`	`10`	`],`
`11`	`11`	`"defaultOptions" : {`
	`12`	`+ "maximumTestRepetitions" : 5,`
`12`	`13`	`"testRepetitionMode" : "retryOnFailure"`
`13`	`14`	`},`
`14`	`15`	`"testTargets" : [`