From aa149f6544e2cfbf64771f80000f55ca9f4378a4 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Fri, 16 Aug 2019 12:18:58 -0300 Subject: [PATCH] Fix broken tests. --- Signal/test/util/YDBToGRDBMigrationTest.swift | 21 +++++++++ .../src/Network/MessageSenderJobQueue.swift | 2 +- .../src/Storage/SSKMessageSenderJobRecord.h | 2 + .../src/Storage/SSKMessageSenderJobRecord.m | 8 +++- .../tests/Messages/OWSUDManagerTest.swift | 4 +- .../Network/MessageSenderJobRecordTest.swift | 43 +++++++++++-------- 6 files changed, 58 insertions(+), 22 deletions(-) diff --git a/Signal/test/util/YDBToGRDBMigrationTest.swift b/Signal/test/util/YDBToGRDBMigrationTest.swift index 10317b1507..f3be20a4e3 100644 --- a/Signal/test/util/YDBToGRDBMigrationTest.swift +++ b/Signal/test/util/YDBToGRDBMigrationTest.swift @@ -19,6 +19,9 @@ class YDBToGRDBMigrationTest: SignalBaseTest { } func testKeyValueInt() { + // GRDB TODO: This test can be fixed using the storage coordinator in master. + XCTAssert(FeatureFlags.useGRDB) + let store1 = SDSKeyValueStore(collection: "store1") let store2 = SDSKeyValueStore(collection: "store2") let store3 = SDSKeyValueStore(collection: "store3") @@ -95,6 +98,9 @@ class YDBToGRDBMigrationTest: SignalBaseTest { } func testKeyValueUInt() { + // GRDB TODO: This test can be fixed using the storage coordinator in master. + XCTAssert(FeatureFlags.useGRDB) + let store1 = SDSKeyValueStore(collection: "store1") let store2 = SDSKeyValueStore(collection: "store2") let store3 = SDSKeyValueStore(collection: "store3") @@ -171,6 +177,9 @@ class YDBToGRDBMigrationTest: SignalBaseTest { } func testKeyValueDouble() { + // GRDB TODO: This test can be fixed using the storage coordinator in master. + XCTAssert(FeatureFlags.useGRDB) + let store1 = SDSKeyValueStore(collection: "store1") let store2 = SDSKeyValueStore(collection: "store2") let store3 = SDSKeyValueStore(collection: "store3") @@ -247,6 +256,9 @@ class YDBToGRDBMigrationTest: SignalBaseTest { } func testKeyValueData() { + // GRDB TODO: This test can be fixed using the storage coordinator in master. + XCTAssert(FeatureFlags.useGRDB) + let store1 = SDSKeyValueStore(collection: "store1") let store2 = SDSKeyValueStore(collection: "store2") let store3 = SDSKeyValueStore(collection: "store3") @@ -315,6 +327,9 @@ class YDBToGRDBMigrationTest: SignalBaseTest { } func testKeyValueString() { + // GRDB TODO: This test can be fixed using the storage coordinator in master. + XCTAssert(FeatureFlags.useGRDB) + let store1 = SDSKeyValueStore(collection: "store1") let store2 = SDSKeyValueStore(collection: "store2") let store3 = SDSKeyValueStore(collection: "store3") @@ -383,6 +398,9 @@ class YDBToGRDBMigrationTest: SignalBaseTest { } func testKeyValueDate() { + // GRDB TODO: This test can be fixed using the storage coordinator in master. + XCTAssert(FeatureFlags.useGRDB) + let store1 = SDSKeyValueStore(collection: "store1") let store2 = SDSKeyValueStore(collection: "store2") let store3 = SDSKeyValueStore(collection: "store3") @@ -470,6 +488,9 @@ class YDBToGRDBMigrationTest: SignalBaseTest { } func testKeyValueObject() { + // GRDB TODO: This test can be fixed using the storage coordinator in master. + XCTAssert(FeatureFlags.useGRDB) + let store1 = SDSKeyValueStore(collection: "store1") let store2 = SDSKeyValueStore(collection: "store2") let store3 = SDSKeyValueStore(collection: "store3") diff --git a/SignalServiceKit/src/Network/MessageSenderJobQueue.swift b/SignalServiceKit/src/Network/MessageSenderJobQueue.swift index 457a7718d3..543c99fe21 100644 --- a/SignalServiceKit/src/Network/MessageSenderJobQueue.swift +++ b/SignalServiceKit/src/Network/MessageSenderJobQueue.swift @@ -67,7 +67,7 @@ public class MessageSenderJobQueue: NSObject, JobQueue { let jobRecord: SSKMessageSenderJobRecord do { - jobRecord = try SSKMessageSenderJobRecord(message: message, removeMessageAfterSending: false, label: self.jobRecordLabel) + jobRecord = try SSKMessageSenderJobRecord(message: message, removeMessageAfterSending: false, label: self.jobRecordLabel, transaction: transaction) } catch { owsFailDebug("failed to build job: \(error)") return diff --git a/SignalServiceKit/src/Storage/SSKMessageSenderJobRecord.h b/SignalServiceKit/src/Storage/SSKMessageSenderJobRecord.h index 377082d1d8..e5527d1306 100644 --- a/SignalServiceKit/src/Storage/SSKMessageSenderJobRecord.h +++ b/SignalServiceKit/src/Storage/SSKMessageSenderJobRecord.h @@ -6,6 +6,7 @@ NS_ASSUME_NONNULL_BEGIN +@class SDSAnyReadTransaction; @class TSOutgoingMessage; @interface SSKMessageSenderJobRecord : SSKJobRecord @@ -18,6 +19,7 @@ NS_ASSUME_NONNULL_BEGIN - (nullable instancetype)initWithMessage:(TSOutgoingMessage *)message removeMessageAfterSending:(BOOL)removeMessageAfterSending label:(NSString *)label + transaction:(SDSAnyReadTransaction *)transaction error:(NSError **)outError NS_DESIGNATED_INITIALIZER; - (nullable instancetype)initWithCoder:(NSCoder *)coder NS_DESIGNATED_INITIALIZER; diff --git a/SignalServiceKit/src/Storage/SSKMessageSenderJobRecord.m b/SignalServiceKit/src/Storage/SSKMessageSenderJobRecord.m index b4e873b9ff..1cc651f223 100644 --- a/SignalServiceKit/src/Storage/SSKMessageSenderJobRecord.m +++ b/SignalServiceKit/src/Storage/SSKMessageSenderJobRecord.m @@ -4,6 +4,7 @@ #import "SSKMessageSenderJobRecord.h" #import "TSOutgoingMessage.h" +#import NS_ASSUME_NONNULL_BEGIN @@ -19,6 +20,7 @@ NS_ASSUME_NONNULL_BEGIN - (nullable instancetype)initWithMessage:(TSOutgoingMessage *)message removeMessageAfterSending:(BOOL)removeMessageAfterSending label:(NSString *)label + transaction:(SDSAnyReadTransaction *)transaction error:(NSError **)outError { self = [super initWithLabel:label]; @@ -28,10 +30,12 @@ NS_ASSUME_NONNULL_BEGIN if (message.shouldBeSaved) { _messageId = message.uniqueId; - if (_messageId == nil) { + OWSAssertDebug(_messageId.length > 0); + BOOL isSaved = [TSInteraction anyFetchWithUniqueId:_messageId transaction:transaction] != nil; + if (!isSaved) { *outError = [NSError errorWithDomain:SSKJobRecordErrorDomain code:JobRecordError_AssertionError - userInfo:@{ NSDebugDescriptionErrorKey : @"messageId wasn't set" }]; + userInfo:@{ NSDebugDescriptionErrorKey : @"message wasn't saved" }]; return nil; } _invisibleMessage = nil; diff --git a/SignalServiceKit/tests/Messages/OWSUDManagerTest.swift b/SignalServiceKit/tests/Messages/OWSUDManagerTest.swift index afc65105e6..0beed845f9 100644 --- a/SignalServiceKit/tests/Messages/OWSUDManagerTest.swift +++ b/SignalServiceKit/tests/Messages/OWSUDManagerTest.swift @@ -50,7 +50,7 @@ class OWSUDManagerTest: SSKBaseTestSwift { // Configure UDManager self.write { transaction in - profileManager.setProfileKeyData(OWSAES256Key.generateRandom().keyData, for: aliceAddress, transaction: transaction) + self.profileManager.setProfileKeyData(OWSAES256Key.generateRandom().keyData, for: self.aliceAddress, transaction: transaction) } udManager.certificateValidator = MockCertificateValidator() @@ -159,7 +159,7 @@ class OWSUDManagerTest: SSKBaseTestSwift { let bobRecipientAddress = SignalServiceAddress(phoneNumber: "+13213214322") XCTAssertFalse(bobRecipientAddress.isLocalAddress) self.write { transaction in - profileManager.setProfileKeyData(OWSAES256Key.generateRandom().keyData, for: bobRecipientAddress, transaction: transaction) + self.profileManager.setProfileKeyData(OWSAES256Key.generateRandom().keyData, for: bobRecipientAddress, transaction: transaction) } var udAccess: OWSUDAccess! diff --git a/SignalServiceKit/tests/Network/MessageSenderJobRecordTest.swift b/SignalServiceKit/tests/Network/MessageSenderJobRecordTest.swift index 7b74d690ce..a894f4941f 100644 --- a/SignalServiceKit/tests/Network/MessageSenderJobRecordTest.swift +++ b/SignalServiceKit/tests/Network/MessageSenderJobRecordTest.swift @@ -11,34 +11,43 @@ class SSKMessageSenderJobRecordTest: SSKBaseTestSwift { func test_savedVisibleMessage() { let message = OutgoingMessageFactory().create() - let jobRecord = try! SSKMessageSenderJobRecord(message: message, removeMessageAfterSending: false, label: MessageSenderJobQueue.jobRecordLabel) - XCTAssertNotNil(jobRecord.messageId) - XCTAssertNotNil(jobRecord.threadId) - XCTAssertNil(jobRecord.invisibleMessage) + self.yapRead { transaction in + let jobRecord = try! SSKMessageSenderJobRecord(message: message, + removeMessageAfterSending: false, + label: MessageSenderJobQueue.jobRecordLabel, + transaction: transaction.asAnyRead) + XCTAssertNotNil(jobRecord.messageId) + XCTAssertNotNil(jobRecord.threadId) + XCTAssertNil(jobRecord.invisibleMessage) + } } func test_unsavedVisibleMessage() { var message: TSOutgoingMessage! self.yapWrite { transaction in message = OutgoingMessageFactory().build(transaction: transaction.asAnyWrite) - } - do { - _ = try SSKMessageSenderJobRecord(message: message, removeMessageAfterSending: false, label: MessageSenderJobQueue.jobRecordLabel) - XCTFail("Should error") - } catch JobRecordError.assertionError { - // expected - } catch { - XCTFail("unexpected error: \(error)") + do { + _ = try SSKMessageSenderJobRecord(message: message, removeMessageAfterSending: false, label: MessageSenderJobQueue.jobRecordLabel, transaction: transaction.asAnyRead) + XCTFail("Should error") + } catch JobRecordError.assertionError { + // expected + } catch { + XCTFail("unexpected error: \(error)") + } } } func test_invisibleMessage() { let message = OutgoingMessageFactory().buildDeliveryReceipt() - - let jobRecord = try! SSKMessageSenderJobRecord(message: message, removeMessageAfterSending: false, label: MessageSenderJobQueue.jobRecordLabel) - XCTAssertNil(jobRecord.messageId) - XCTAssertNotNil(jobRecord.threadId) - XCTAssertNotNil(jobRecord.invisibleMessage) + self.yapRead { transaction in + let jobRecord = try! SSKMessageSenderJobRecord(message: message, + removeMessageAfterSending: false, + label: MessageSenderJobQueue.jobRecordLabel, + transaction: transaction.asAnyRead) + XCTAssertNil(jobRecord.messageId) + XCTAssertNotNil(jobRecord.threadId) + XCTAssertNotNil(jobRecord.invisibleMessage) + } } }