From f307f480659b33af961b54704945373e3f807f56 Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Mon, 5 Jun 2023 15:29:44 +1000 Subject: [PATCH 1/4] Add `WPAccount` fixture utility This is part of an effort to make `AccountSettingsServiceTests.testUpdateFailure()` less flaky in CI. --- WordPress/WordPress.xcodeproj/project.pbxproj | 24 +++++----- .../WordPressTest/AccountServiceTests.swift | 33 +++----------- .../AccountSettingsServiceTests.swift | 6 +-- WordPress/WordPressTest/BlogTests.swift | 4 +- .../WordPressTest/ContextManagerTests.swift | 44 +++++-------------- .../WordPressTest/PeopleServiceTests.swift | 4 +- .../WordPressTest/WPAccount+Fixture.swift | 20 +++++++++ 7 files changed, 54 insertions(+), 81 deletions(-) create mode 100644 WordPress/WordPressTest/WPAccount+Fixture.swift diff --git a/WordPress/WordPress.xcodeproj/project.pbxproj b/WordPress/WordPress.xcodeproj/project.pbxproj index 6cfebe3c1810..abf3845cd057 100644 --- a/WordPress/WordPress.xcodeproj/project.pbxproj +++ b/WordPress/WordPress.xcodeproj/project.pbxproj @@ -862,6 +862,7 @@ 3F73BE5F24EB3B4400BE99FF /* WhatIsNewView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3F73BE5E24EB3B4400BE99FF /* WhatIsNewView.swift */; }; 3F751D462491A93D0008A2B1 /* ZendeskUtilsTests+Plans.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3F751D452491A93D0008A2B1 /* ZendeskUtilsTests+Plans.swift */; }; 3F758FD524F6FB4900BBA2FC /* AnnouncementsStore.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3F758FD424F6FB4900BBA2FC /* AnnouncementsStore.swift */; }; + 3F759FBA2A2DA93B0039A845 /* WPAccount+Fixture.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3F759FB92A2DA93B0039A845 /* WPAccount+Fixture.swift */; }; 3F762E9326784A950088CD45 /* Logger.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3F762E9226784A950088CD45 /* Logger.swift */; }; 3F762E9526784B540088CD45 /* WireMock.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3F762E9426784B540088CD45 /* WireMock.swift */; }; 3F762E9726784BED0088CD45 /* FancyAlertComponent.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3F762E9626784BED0088CD45 /* FancyAlertComponent.swift */; }; @@ -6560,6 +6561,7 @@ 3F73BE5E24EB3B4400BE99FF /* WhatIsNewView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WhatIsNewView.swift; sourceTree = ""; }; 3F751D452491A93D0008A2B1 /* ZendeskUtilsTests+Plans.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "ZendeskUtilsTests+Plans.swift"; sourceTree = ""; }; 3F758FD424F6FB4900BBA2FC /* AnnouncementsStore.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = AnnouncementsStore.swift; sourceTree = ""; }; + 3F759FB92A2DA93B0039A845 /* WPAccount+Fixture.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "WPAccount+Fixture.swift"; sourceTree = ""; }; 3F762E9226784A950088CD45 /* Logger.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Logger.swift; sourceTree = ""; }; 3F762E9426784B540088CD45 /* WireMock.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WireMock.swift; sourceTree = ""; }; 3F762E9626784BED0088CD45 /* FancyAlertComponent.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FancyAlertComponent.swift; sourceTree = ""; }; @@ -12374,26 +12376,27 @@ 5D7A577D1AFBFD7C0097C028 /* Models */ = { isa = PBXGroup; children = ( - F1B1E7A224098FA100549E2A /* BlogTests.swift */, + E6A2158F1D1065F200DE5270 /* AbstractPostTest.swift */, + 8B8C814C2318073300A0E620 /* BasePostTests.swift */, 246D0A0225E97D5D0028B83F /* Blog+ObjcTests.m */, + FEE48EFE2A4C9855008A48E0 /* Blog+PublicizeTests.swift */, 4AD5657128E543A30054C676 /* BlogQueryTests.swift */, + B55F1AA11C107CE200FD04D4 /* BlogSettingsDiscussionTests.swift */, + F1B1E7A224098FA100549E2A /* BlogTests.swift */, + 24A2948225D602710000A51E /* BlogTimeZoneTests.m */, D848CC1620FF38EA00A9038F /* FormattableCommentRangeTests.swift */, + 0879FC151E9301DD00E1EFC8 /* MediaTests.swift */, + C38C5D8027F61D2C002F517E /* MenuItemTests.swift */, D848CC1420FF33FC00A9038F /* NotificationContentRangeTests.swift */, + D826D67E211D21C700A5D8FE /* NullMockUserDefaults.swift */, + 5960967E1CF7959300848496 /* PostTests.swift */, 8BBBEBB124B8F8C0005E358E /* ReaderCardTests.swift */, E6B9B8A91B94E1FE0001B92F /* ReaderPostTest.m */, - B55F1AA11C107CE200FD04D4 /* BlogSettingsDiscussionTests.swift */, - E6A2158F1D1065F200DE5270 /* AbstractPostTest.swift */, - 5960967E1CF7959300848496 /* PostTests.swift */, - 0879FC151E9301DD00E1EFC8 /* MediaTests.swift */, - D826D67E211D21C700A5D8FE /* NullMockUserDefaults.swift */, - 8B8C814C2318073300A0E620 /* BasePostTests.swift */, - 24A2948225D602710000A51E /* BlogTimeZoneTests.m */, 24C69A8A2612421900312D9A /* UserSettingsTests.swift */, 24C69AC12612467C00312D9A /* UserSettingsTestsObjc.m */, + 3F759FB92A2DA93B0039A845 /* WPAccount+Fixture.swift */, 2481B1E7260D4EAC00AE59DB /* WPAccount+LookupTests.swift */, 2481B20B260D8FED00AE59DB /* WPAccount+ObjCLookupTests.m */, - C38C5D8027F61D2C002F517E /* MenuItemTests.swift */, - FEE48EFE2A4C9855008A48E0 /* Blog+PublicizeTests.swift */, ); name = Models; sourceTree = ""; @@ -23630,6 +23633,7 @@ F17A2A2023BFBD84001E96AC /* UIView+ExistingConstraints.swift in Sources */, 9A9D34FD23607CCC00BC95A3 /* AsyncOperationTests.swift in Sources */, B5552D821CD1061F00B26DF6 /* StringExtensionsTests.swift in Sources */, + 3F759FBA2A2DA93B0039A845 /* WPAccount+Fixture.swift in Sources */, 8B821F3C240020E2006B697E /* PostServiceUploadingListTests.swift in Sources */, 73178C3521BEE9AC00E37C9A /* TitleSubtitleHeaderTests.swift in Sources */, 08AAD6A11CBEA610002B2418 /* MenusServiceTests.m in Sources */, diff --git a/WordPress/WordPressTest/AccountServiceTests.swift b/WordPress/WordPressTest/AccountServiceTests.swift index d41409d8f705..ba0abe17782c 100644 --- a/WordPress/WordPressTest/AccountServiceTests.swift +++ b/WordPress/WordPressTest/AccountServiceTests.swift @@ -147,24 +147,10 @@ class AccountServiceTests: CoreDataTestCase { func testMergeMultipleDuplicateAccounts() throws { let context = contextManager.mainContext - let account1 = WPAccount(context: context) - account1.userID = 1 - account1.username = "username" - account1.authToken = "authToken" - account1.uuid = UUID().uuidString - - let account2 = WPAccount(context: context) - account2.userID = 1 - account2.username = "username" - account2.authToken = "authToken" - account2.uuid = UUID().uuidString - - let account3 = WPAccount(context: context) - account3.userID = 1 - account3.username = "username" - account3.authToken = "authToken" - account3.uuid = UUID().uuidString + let account1 = WPAccount.fixture(context: context, userID: 1) + let account2 = WPAccount.fixture(context: context, userID: 1) + let account3 = WPAccount.fixture(context: context, userID: 1) account1.addBlogs(createMockBlogs(withIDs: [1, 2, 3, 4, 5, 6], in: context)) account2.addBlogs(createMockBlogs(withIDs: [1, 2, 3], in: context)) @@ -240,17 +226,8 @@ class AccountServiceTests: CoreDataTestCase { } func testPurgeAccount() throws { - let account1 = WPAccount(context: mainContext) - account1.userID = 1 - account1.username = "username" - account1.authToken = "authToken" - account1.uuid = UUID().uuidString - - let account2 = WPAccount(context: mainContext) - account2.userID = 1 - account2.username = "username" - account2.authToken = "authToken" - account2.uuid = UUID().uuidString + let account1 = WPAccount.fixture(context: mainContext, userID: 1) + let account2 = WPAccount.fixture(context: mainContext, userID: 2) contextManager.saveContextAndWait(mainContext) try XCTAssertEqual(mainContext.count(for: WPAccount.fetchRequest()), 2) diff --git a/WordPress/WordPressTest/AccountSettingsServiceTests.swift b/WordPress/WordPressTest/AccountSettingsServiceTests.swift index b365eb39fa6d..6f4dbccdcae5 100644 --- a/WordPress/WordPressTest/AccountSettingsServiceTests.swift +++ b/WordPress/WordPressTest/AccountSettingsServiceTests.swift @@ -8,11 +8,7 @@ class AccountSettingsServiceTests: CoreDataTestCase { private var service: AccountSettingsService! override func setUp() { - let account = WPAccount(context: mainContext) - account.username = "test" - account.authToken = "token" - account.userID = 1 - account.uuid = UUID().uuidString + let account = WPAccount.fixture(context: mainContext) let settings = ManagedAccountSettings(context: mainContext) settings.account = account diff --git a/WordPress/WordPressTest/BlogTests.swift b/WordPress/WordPressTest/BlogTests.swift index c5048fdc2f80..b50b72a9f566 100644 --- a/WordPress/WordPressTest/BlogTests.swift +++ b/WordPress/WordPressTest/BlogTests.swift @@ -199,9 +199,7 @@ final class BlogTests: CoreDataTestCase { // Create an account with duplicated blogs let xmlrpc = "https://xmlrpc.test.wordpress.com" let account = try await contextManager.performAndSave { context in - let account = WPAccount(context: context) - account.username = "username" - account.authToken = "authToken" + let account = WPAccount.fixture(context: context) account.blogs = Set( (1...10).map { _ in let blog = BlogBuilder(context).build() diff --git a/WordPress/WordPressTest/ContextManagerTests.swift b/WordPress/WordPressTest/ContextManagerTests.swift index 5a9f0f16a3f6..c9268ab92fcb 100644 --- a/WordPress/WordPressTest/ContextManagerTests.swift +++ b/WordPress/WordPressTest/ContextManagerTests.swift @@ -146,9 +146,7 @@ class ContextManagerTests: XCTestCase { let derivedContext = contextManager.newDerivedContext() derivedContext.perform { - let account = WPAccount(context: derivedContext) - account.userID = 1 - account.username = "First User" + _ = WPAccount.fixture(context: derivedContext, userID: 1, username: "First User") contextManager.saveContextAndWait(derivedContext) } @@ -165,9 +163,7 @@ class ContextManagerTests: XCTestCase { // Save another user waitUntil { done in derivedContext.perform { - let account = WPAccount(context: derivedContext) - account.userID = 2 - account.username = "Second account" + _ = WPAccount.fixture(context: derivedContext, userID: 2) contextManager.saveContextAndWait(derivedContext) done() } @@ -193,17 +189,13 @@ class ContextManagerTests: XCTestCase { XCTAssertEqual(numberOfAccounts(), 0) try await contextManager.performAndSave { context in - let account = WPAccount(context: context) - account.userID = 1 - account.username = "First User" + _ = WPAccount.fixture(context: context, userID: 1) } XCTAssertEqual(numberOfAccounts(), 1) do { try await contextManager.performAndSave { context in - let account = WPAccount(context: context) - account.userID = 100 - account.username = "Unknown User" + _ = WPAccount.fixture(context: context, userID: 100) throw NSError(domain: "save", code: 1) } XCTFail("The above call should throw") @@ -221,9 +213,7 @@ class ContextManagerTests: XCTestCase { // > "In non-async functions, and closures without any await expression, the compiler selects the non-async overload" let sync: () -> Void = { contextManager.performAndSave { context in - let account = WPAccount(context: context) - account.userID = 2 - account.username = "Second User" + _ = WPAccount.fixture(context: context, userID: 2) } } sync() @@ -245,14 +235,10 @@ class ContextManagerTests: XCTestCase { ] contextManager.performAndSave({ - let account = WPAccount(context: $0) - account.userID = 1 - account.username = "First User" + _ = WPAccount.fixture(context: $0, userID: 1, username: "First User") contextManager.performAndSave { - let account = WPAccount(context: $0) - account.userID = 2 - account.username = "Second User" + _ = WPAccount.fixture(context: $0, userID: 2, username: "Second User") } saveOperations[1].fulfill() @@ -279,14 +265,10 @@ class ContextManagerTests: XCTestCase { ] contextManager.performAndSave({ - let account = WPAccount(context: $0) - account.userID = 1 - account.username = "First User" + _ = WPAccount.fixture(context: $0, userID: 1, username: "First User") contextManager.performAndSave({ - let account = WPAccount(context: $0) - account.userID = 2 - account.username = "Second User" + _ = WPAccount.fixture(context: $0, userID: 2, username: "Second User") }, completion: { saveOperations[1].fulfill() }, on: .main) @@ -380,9 +362,7 @@ class ContextManagerTests: XCTestCase { // First, insert an account into the database. let contextManager = ContextManager.forTesting() contextManager.performAndSave { context in - let account = WPAccount(context: context) - account.userID = 1 - account.username = "First User" + _ = WPAccount.fixture(context: context, userID: 1, username: "First User") } // Fetch the account in the main context @@ -441,8 +421,8 @@ class ContextManagerTests: XCTestCase { private func createOrUpdateAccount(username: String, newToken: String, in context: NSManagedObjectContext) throws { var account = try WPAccount.lookup(withUsername: username, in: context) if account == nil { - account = WPAccount(context: context) - account?.username = username + // Will this make tests fail because of the default userID in the fixture? + account = WPAccount.fixture(context: context, username: username) } account?.authToken = newToken } diff --git a/WordPress/WordPressTest/PeopleServiceTests.swift b/WordPress/WordPressTest/PeopleServiceTests.swift index 128a27306586..48d46b3bee5c 100644 --- a/WordPress/WordPressTest/PeopleServiceTests.swift +++ b/WordPress/WordPressTest/PeopleServiceTests.swift @@ -18,9 +18,7 @@ class PeopleServiceTests: CoreDataTestCase { } contextManager.performAndSave { context in - let account = WPAccount(context: context) - account.username = "username" - account.authToken = "token" + let account = WPAccount.fixture(context: context) let blog = Blog(context: context) blog.dotComID = NSNumber(value: self.siteID) diff --git a/WordPress/WordPressTest/WPAccount+Fixture.swift b/WordPress/WordPressTest/WPAccount+Fixture.swift new file mode 100644 index 000000000000..c56b8c08df0e --- /dev/null +++ b/WordPress/WordPressTest/WPAccount+Fixture.swift @@ -0,0 +1,20 @@ +/// Centralized utility to generate preconfigured WPAccount instances +extension WPAccount { + + static func fixture( + context: NSManagedObjectContext, + // Using a constant UUID by default to keep the tests deterministic. + // There's nothing special in the value itself. It's just a UUID() value copied over. + uuid: UUID = UUID(uuidString: "D0D0298F-D7EF-4F32-A1F8-DDDBB8ADB8DF")!, + userID: Int = 1, + username: String = "username", + authToken: String = "authToken" + ) -> WPAccount { + let account = WPAccount(context: context) + account.userID = NSNumber(value: userID) + account.username = username + account.authToken = authToken + account.uuid = uuid.uuidString + return account + } +} From aae784b80ffb33f60923f280f91288d76f9593fd Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Mon, 5 Jun 2023 16:06:50 +1000 Subject: [PATCH 2/4] Define `TestError`, an error type for tests --- WordPress/WordPress.xcodeproj/project.pbxproj | 12 ++++++++---- WordPress/WordPressTest/TestError.swift | 8 ++++++++ 2 files changed, 16 insertions(+), 4 deletions(-) create mode 100644 WordPress/WordPressTest/TestError.swift diff --git a/WordPress/WordPress.xcodeproj/project.pbxproj b/WordPress/WordPress.xcodeproj/project.pbxproj index abf3845cd057..0d179d7ef7dd 100644 --- a/WordPress/WordPress.xcodeproj/project.pbxproj +++ b/WordPress/WordPress.xcodeproj/project.pbxproj @@ -863,6 +863,7 @@ 3F751D462491A93D0008A2B1 /* ZendeskUtilsTests+Plans.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3F751D452491A93D0008A2B1 /* ZendeskUtilsTests+Plans.swift */; }; 3F758FD524F6FB4900BBA2FC /* AnnouncementsStore.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3F758FD424F6FB4900BBA2FC /* AnnouncementsStore.swift */; }; 3F759FBA2A2DA93B0039A845 /* WPAccount+Fixture.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3F759FB92A2DA93B0039A845 /* WPAccount+Fixture.swift */; }; + 3F759FBC2A2DB2CF0039A845 /* TestError.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3F759FBB2A2DB2CF0039A845 /* TestError.swift */; }; 3F762E9326784A950088CD45 /* Logger.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3F762E9226784A950088CD45 /* Logger.swift */; }; 3F762E9526784B540088CD45 /* WireMock.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3F762E9426784B540088CD45 /* WireMock.swift */; }; 3F762E9726784BED0088CD45 /* FancyAlertComponent.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3F762E9626784BED0088CD45 /* FancyAlertComponent.swift */; }; @@ -6562,6 +6563,7 @@ 3F751D452491A93D0008A2B1 /* ZendeskUtilsTests+Plans.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "ZendeskUtilsTests+Plans.swift"; sourceTree = ""; }; 3F758FD424F6FB4900BBA2FC /* AnnouncementsStore.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = AnnouncementsStore.swift; sourceTree = ""; }; 3F759FB92A2DA93B0039A845 /* WPAccount+Fixture.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "WPAccount+Fixture.swift"; sourceTree = ""; }; + 3F759FBB2A2DB2CF0039A845 /* TestError.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TestError.swift; sourceTree = ""; }; 3F762E9226784A950088CD45 /* Logger.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Logger.swift; sourceTree = ""; }; 3F762E9426784B540088CD45 /* WireMock.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WireMock.swift; sourceTree = ""; }; 3F762E9626784BED0088CD45 /* FancyAlertComponent.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FancyAlertComponent.swift; sourceTree = ""; }; @@ -12212,13 +12214,14 @@ 59B48B601B99E0B0008EBB84 /* TestUtilities */ = { isa = PBXGroup; children = ( - 59B48B611B99E132008EBB84 /* JSONObject.swift */, - E157D5DF1C690A6C00F04FB9 /* ImmuTableTestUtils.swift */, - 570BFD8F2282418A007859A8 /* PostBuilder.swift */, + 2481B1D4260D4E8B00AE59DB /* AccountBuilder.swift */, 57B71D4D230DB5F200789A68 /* BlogBuilder.swift */, + E157D5DF1C690A6C00F04FB9 /* ImmuTableTestUtils.swift */, + 59B48B611B99E132008EBB84 /* JSONObject.swift */, F11023A223186BCA00C4E84A /* MediaBuilder.swift */, 57889AB723589DF100DAE56D /* PageBuilder.swift */, - 2481B1D4260D4E8B00AE59DB /* AccountBuilder.swift */, + 570BFD8F2282418A007859A8 /* PostBuilder.swift */, + 3F759FBB2A2DB2CF0039A845 /* TestError.swift */, ); name = TestUtilities; sourceTree = ""; @@ -23528,6 +23531,7 @@ FF9A6E7121F9361700D36D14 /* MediaUploadHashTests.swift in Sources */, B532ACCF1DC3AB8E00FFFA57 /* NotificationSyncMediatorTests.swift in Sources */, 7E4A772F20F7FDF8001C706D /* ActivityLogTestData.swift in Sources */, + 3F759FBC2A2DB2CF0039A845 /* TestError.swift in Sources */, 40E7FEC82211EEC00032834E /* LastPostStatsRecordValueTests.swift in Sources */, 57240224234E5BE200227067 /* PostServiceSelfHostedTests.swift in Sources */, B59D40A61DB522DF003D2D79 /* NSAttributedStringTests.swift in Sources */, diff --git a/WordPress/WordPressTest/TestError.swift b/WordPress/WordPressTest/TestError.swift new file mode 100644 index 000000000000..417e1b1162d7 --- /dev/null +++ b/WordPress/WordPressTest/TestError.swift @@ -0,0 +1,8 @@ +struct TestError: Error { + + let id: Int + + init(id: Int = 1) { + self.id = id + } +} From 0d947e77815b47e54720031b44c1dc8e1853b6fe Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Mon, 5 Jun 2023 16:24:47 +1000 Subject: [PATCH 3/4] Use a test double over HTTP stub in `testUpdateFailure` --- WordPress/WordPress.xcodeproj/project.pbxproj | 4 + .../AccountSettingsRemoteInterfaceStub.swift | 77 +++++++++++++++++ .../AccountSettingsServiceTests.swift | 82 ++++++++++++------- 3 files changed, 134 insertions(+), 29 deletions(-) create mode 100644 WordPress/WordPressTest/AccountSettingsRemoteInterfaceStub.swift diff --git a/WordPress/WordPress.xcodeproj/project.pbxproj b/WordPress/WordPress.xcodeproj/project.pbxproj index 0d179d7ef7dd..1437352faa27 100644 --- a/WordPress/WordPress.xcodeproj/project.pbxproj +++ b/WordPress/WordPress.xcodeproj/project.pbxproj @@ -864,6 +864,7 @@ 3F758FD524F6FB4900BBA2FC /* AnnouncementsStore.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3F758FD424F6FB4900BBA2FC /* AnnouncementsStore.swift */; }; 3F759FBA2A2DA93B0039A845 /* WPAccount+Fixture.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3F759FB92A2DA93B0039A845 /* WPAccount+Fixture.swift */; }; 3F759FBC2A2DB2CF0039A845 /* TestError.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3F759FBB2A2DB2CF0039A845 /* TestError.swift */; }; + 3F759FBE2A2DB3280039A845 /* AccountSettingsRemoteInterfaceStub.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3F759FBD2A2DB3280039A845 /* AccountSettingsRemoteInterfaceStub.swift */; }; 3F762E9326784A950088CD45 /* Logger.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3F762E9226784A950088CD45 /* Logger.swift */; }; 3F762E9526784B540088CD45 /* WireMock.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3F762E9426784B540088CD45 /* WireMock.swift */; }; 3F762E9726784BED0088CD45 /* FancyAlertComponent.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3F762E9626784BED0088CD45 /* FancyAlertComponent.swift */; }; @@ -6564,6 +6565,7 @@ 3F758FD424F6FB4900BBA2FC /* AnnouncementsStore.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = AnnouncementsStore.swift; sourceTree = ""; }; 3F759FB92A2DA93B0039A845 /* WPAccount+Fixture.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "WPAccount+Fixture.swift"; sourceTree = ""; }; 3F759FBB2A2DB2CF0039A845 /* TestError.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TestError.swift; sourceTree = ""; }; + 3F759FBD2A2DB3280039A845 /* AccountSettingsRemoteInterfaceStub.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AccountSettingsRemoteInterfaceStub.swift; sourceTree = ""; }; 3F762E9226784A950088CD45 /* Logger.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Logger.swift; sourceTree = ""; }; 3F762E9426784B540088CD45 /* WireMock.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WireMock.swift; sourceTree = ""; }; 3F762E9626784BED0088CD45 /* FancyAlertComponent.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FancyAlertComponent.swift; sourceTree = ""; }; @@ -15387,6 +15389,7 @@ children = ( 9363113E19FA996700B0C739 /* AccountServiceTests.swift */, 4A9948E129714EF1006282A9 /* AccountSettingsServiceTests.swift */, + 3F759FBD2A2DB3280039A845 /* AccountSettingsRemoteInterfaceStub.swift */, F1F083F5241FFE930056D3B1 /* AtomicAuthenticationServiceTests.swift */, 46F58500262605930010A723 /* BlockEditorSettingsServiceTests.swift */, FEA312832987FB0100FFD405 /* BlogJetpackTests.swift */, @@ -23519,6 +23522,7 @@ 7E987F58210811CC00CAFB88 /* NotificationContentRouterTests.swift in Sources */, E1C9AA561C10427100732665 /* MathTest.swift in Sources */, 93A379EC19FFBF7900415023 /* KeychainTest.m in Sources */, + 3F759FBE2A2DB3280039A845 /* AccountSettingsRemoteInterfaceStub.swift in Sources */, F1BB660C274E704D00A319BE /* LikeUserHelperTests.swift in Sources */, 436D5655212209D600CEAA33 /* RegisterDomainDetailsServiceProxyMock.swift in Sources */, F1450CF92437EEBB00A28BFE /* MediaRequestAuthenticatorTests.swift in Sources */, diff --git a/WordPress/WordPressTest/AccountSettingsRemoteInterfaceStub.swift b/WordPress/WordPressTest/AccountSettingsRemoteInterfaceStub.swift new file mode 100644 index 000000000000..59cd089c1887 --- /dev/null +++ b/WordPress/WordPressTest/AccountSettingsRemoteInterfaceStub.swift @@ -0,0 +1,77 @@ +@testable import WordPress +import WordPressKit + +class AccountSettingsRemoteInterfaceStub: AccountSettingsRemoteInterface { + + let updateSettingResult: Result<(), Error> + let getSettingsResult: Result + let changeUsernameShouldSucceed: Bool + let suggestUsernamesResult: [String] + let updatePasswordResult: Result<(), Error> + let closeAccountResult: Result<(), Error> + + init( + updateSettingResult: Result = .success(()), + // Defaulting to failure to avoid having to create AccountSettings here, because it required an NSManagedContext + getSettingsResult: Result = .failure(TestError()), + changeUsernameShouldSucceed: Bool = true, + suggestUsernamesResult: [String] = [], + updatePasswordResult: Result = .success(()), + closeAccountResult: Result = .success(()) + ) { + self.updateSettingResult = updateSettingResult + self.getSettingsResult = getSettingsResult + self.changeUsernameShouldSucceed = changeUsernameShouldSucceed + self.suggestUsernamesResult = suggestUsernamesResult + self.updatePasswordResult = updatePasswordResult + self.closeAccountResult = closeAccountResult + } + + func updateSetting(_ change: AccountSettingsChange, success: @escaping () -> Void, failure: @escaping (Error) -> Void) { + switch updateSettingResult { + case .success: + success() + case .failure(let error): + failure(error) + } + } + + func getSettings(success: @escaping (WordPressKit.AccountSettings) -> Void, failure: @escaping (Error) -> Void) { + switch getSettingsResult { + case .success(let settings): + success(settings) + case .failure(let error): + failure(error) + } + } + + func changeUsername(to username: String, success: @escaping () -> Void, failure: @escaping () -> Void) { + if changeUsernameShouldSucceed { + success() + } else { + failure() + } + } + + func suggestUsernames(base: String, finished: @escaping ([String]) -> Void) { + finished(suggestUsernamesResult) + } + + func updatePassword(_ password: String, success: @escaping () -> Void, failure: @escaping (Error) -> Void) { + switch updatePasswordResult { + case .success: + success() + case .failure(let error): + failure(error) + } + } + + func closeAccount(success: @escaping () -> Void, failure: @escaping (Error) -> Void) { + switch closeAccountResult { + case .success: + success() + case .failure(let error): + failure(error) + } + } +} diff --git a/WordPress/WordPressTest/AccountSettingsServiceTests.swift b/WordPress/WordPressTest/AccountSettingsServiceTests.swift index 6f4dbccdcae5..427963a3ff25 100644 --- a/WordPress/WordPressTest/AccountSettingsServiceTests.swift +++ b/WordPress/WordPressTest/AccountSettingsServiceTests.swift @@ -9,20 +9,9 @@ class AccountSettingsServiceTests: CoreDataTestCase { override func setUp() { let account = WPAccount.fixture(context: mainContext) - - let settings = ManagedAccountSettings(context: mainContext) - settings.account = account - settings.username = "Username" - settings.displayName = "Display Name" - settings.primarySiteID = 1 - settings.aboutMe = "" - settings.email = "test@email.com" - settings.firstName = "Test" - settings.lastName = "User" - settings.language = "en" - settings.webAddress = "https://test.wordpress.com" - + _ = makeManagedAccountSettings(context: mainContext, account: account) contextManager.saveContextAndWait(mainContext) + service = makeService(contextManager: contextManager, account: account) service = AccountSettingsService( userID: account.userID.intValue, @@ -31,18 +20,6 @@ class AccountSettingsServiceTests: CoreDataTestCase { ) } - private func managedAccountSettings() -> ManagedAccountSettings? { - contextManager.performQuery { context in - let request = NSFetchRequest(entityName: ManagedAccountSettings.entityName()) - request.predicate = NSPredicate(format: "account.userID = %d", self.service.userID) - request.fetchLimit = 1 - guard let results = (try? context.fetch(request)) as? [ManagedAccountSettings] else { - return nil - } - return results.first - } - } - func testUpdateSuccess() throws { stub(condition: isPath("/rest/v1.1/me/settings")) { _ in HTTPStubsResponse(jsonObject: [String: Any](), statusCode: 200, headers: nil) @@ -57,15 +34,21 @@ class AccountSettingsServiceTests: CoreDataTestCase { } func testUpdateFailure() throws { - stub(condition: isPath("/rest/v1.1/me/settings")) { _ in - HTTPStubsResponse(jsonObject: [String: Any](), statusCode: 500, headers: nil) - } + // We've seen some flakiness in CI on this test, and therefore are using a stub object rather than stubbing the HTTP requests. + // Since this approach bypasses the entire network stack, the hope is that it'll result in a more robust test. + let service = AccountSettingsService( + userID: 1, + remote: AccountSettingsRemoteInterfaceStub(updateSettingResult: .failure(TestError())), + coreDataStack: contextManager + ) + waitUntil { done in - self.service.saveChange(.firstName("Updated"), finished: { success in + service.saveChange(.firstName("Updated"), finished: { success in expect(success).to(beFalse()) done() }) } + expect(self.managedAccountSettings()?.firstName).to(equal("Test")) } @@ -96,3 +79,44 @@ class AccountSettingsServiceTests: CoreDataTestCase { wait(for: [notCrash], timeout: 0.5) } } + +extension AccountSettingsServiceTests { + + private func makeManagedAccountSettings( + context: NSManagedObjectContext, + account: WPAccount + ) -> ManagedAccountSettings { + let settings = ManagedAccountSettings(context: context) + settings.account = account + settings.username = "Username" + settings.displayName = "Display Name" + settings.primarySiteID = 1 + settings.aboutMe = "" + settings.email = "test@email.com" + settings.firstName = "Test" + settings.lastName = "User" + settings.language = "en" + settings.webAddress = "https://test.wordpress.com" + return settings + } + + private func makeService(contextManager: ContextManager, account: WPAccount) -> AccountSettingsService { + AccountSettingsService( + userID: account.userID.intValue, + remote: AccountSettingsRemote(wordPressComRestApi: account.wordPressComRestApi), + coreDataStack: contextManager + ) + } + + private func managedAccountSettings() -> ManagedAccountSettings? { + contextManager.performQuery { context in + let request = NSFetchRequest(entityName: ManagedAccountSettings.entityName()) + request.predicate = NSPredicate(format: "account.userID = %d", self.service.userID) + request.fetchLimit = 1 + guard let results = (try? context.fetch(request)) as? [ManagedAccountSettings] else { + return nil + } + return results.first + } + } +} From d244fdcfa5a4b8b2ee343fd9b5eb6b281d2af648 Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Mon, 5 Jun 2023 20:21:04 +1000 Subject: [PATCH 4/4] Use a test double over HTTP stubs in `testUpdateSuccess` --- .../AccountSettingsServiceTests.swift | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/WordPress/WordPressTest/AccountSettingsServiceTests.swift b/WordPress/WordPressTest/AccountSettingsServiceTests.swift index 427963a3ff25..0681613fed44 100644 --- a/WordPress/WordPressTest/AccountSettingsServiceTests.swift +++ b/WordPress/WordPressTest/AccountSettingsServiceTests.swift @@ -21,15 +21,24 @@ class AccountSettingsServiceTests: CoreDataTestCase { } func testUpdateSuccess() throws { - stub(condition: isPath("/rest/v1.1/me/settings")) { _ in - HTTPStubsResponse(jsonObject: [String: Any](), statusCode: 200, headers: nil) - } + // We've seen some flakiness in CI on this test, and therefore are using a stub object rather than stubbing the HTTP requests. + // Since this approach bypasses the entire network stack, the hope is that it'll result in a more robust test. + // + // This is the second test in this class edited this way. + // If we'll need to update a third, we shall also take the time to update the rest of the tests. + let service = AccountSettingsService( + userID: 1, + remote: AccountSettingsRemoteInterfaceStub(updateSettingResult: .success(())), + coreDataStack: contextManager + ) + waitUntil { done in - self.service.saveChange(.firstName("Updated"), finished: { success in + service.saveChange(.firstName("Updated"), finished: { success in expect(success).to(beTrue()) done() }) } + expect(self.managedAccountSettings()?.firstName).to(equal("Updated")) }