From 7585168d3dcb9f4dfc2fd8ed71f052377a87f5f2 Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Fri, 17 Aug 2018 15:39:46 -0700 Subject: [PATCH 1/2] Update json files --- .../SpecTests/json/listen_spec_test.json | 14 ++- .../SpecTests/json/offline_spec_test.json | 92 +++++-------------- .../SpecTests/json/persistence_spec_test.json | 16 +++- .../json/remote_store_spec_test.json | 31 ++++++- .../Tests/SpecTests/json/write_spec_test.json | 2 +- 5 files changed, 75 insertions(+), 80 deletions(-) diff --git a/Firestore/Example/Tests/SpecTests/json/listen_spec_test.json b/Firestore/Example/Tests/SpecTests/json/listen_spec_test.json index 54144a6b610..0c17a572694 100644 --- a/Firestore/Example/Tests/SpecTests/json/listen_spec_test.json +++ b/Firestore/Example/Tests/SpecTests/json/listen_spec_test.json @@ -3,7 +3,7 @@ "describeName": "Listens:", "itName": "Contents of query are cleared when listen is removed.", "tags": [ - "no-lru" + "eager-gc" ], "comment": "Explicitly tests eager GC behavior", "config": { @@ -1287,7 +1287,9 @@ "Will gracefully handle watch stream reverting snapshots (with restart)": { "describeName": "Listens:", "itName": "Will gracefully handle watch stream reverting snapshots (with restart)", - "tags": [], + "tags": [ + "durable-persistence" + ], "config": { "useGarbageCollection": false, "numClients": 1 @@ -4248,7 +4250,9 @@ "Omits global resume tokens for a short while": { "describeName": "Listens:", "itName": "Omits global resume tokens for a short while", - "tags": [], + "tags": [ + "durable-persistence" + ], "config": { "useGarbageCollection": false, "numClients": 1 @@ -4423,7 +4427,9 @@ "Persists global resume tokens if the snapshot is old enough": { "describeName": "Listens:", "itName": "Persists global resume tokens if the snapshot is old enough", - "tags": [], + "tags": [ + "durable-persistence" + ], "config": { "useGarbageCollection": false, "numClients": 1 diff --git a/Firestore/Example/Tests/SpecTests/json/offline_spec_test.json b/Firestore/Example/Tests/SpecTests/json/offline_spec_test.json index 68813ce3764..c0d096606ca 100644 --- a/Firestore/Example/Tests/SpecTests/json/offline_spec_test.json +++ b/Firestore/Example/Tests/SpecTests/json/offline_spec_test.json @@ -2,7 +2,10 @@ "Empty queries are resolved if client goes offline": { "describeName": "Offline:", "itName": "Empty queries are resolved if client goes offline", - "tags": [], + "tags": [ + "no-android", + "no-ios" + ], "config": { "useGarbageCollection": true, "numClients": 1 @@ -30,15 +33,6 @@ } } }, - { - "watchStreamClose": { - "error": { - "code": 14, - "message": "Simulated Backend Error" - }, - "runBackoffTimer": true - } - }, { "watchStreamClose": { "error": { @@ -83,7 +77,10 @@ "A successful message delays offline status": { "describeName": "Offline:", "itName": "A successful message delays offline status", - "tags": [], + "tags": [ + "no-android", + "no-ios" + ], "config": { "useGarbageCollection": true, "numClients": 1 @@ -125,15 +122,6 @@ "runBackoffTimer": true } }, - { - "watchStreamClose": { - "error": { - "code": 14, - "message": "Simulated Backend Error" - }, - "runBackoffTimer": true - } - }, { "watchStreamClose": { "error": { @@ -179,7 +167,9 @@ "describeName": "Offline:", "itName": "Removing all listeners delays \"Offline\" status on next listen", "tags": [ - "no-lru" + "eager-gc", + "no-android", + "no-ios" ], "comment": "Marked as no-lru because when a listen is re-added, it gets a new target id rather than reusing one", "config": { @@ -209,15 +199,6 @@ } } }, - { - "watchStreamClose": { - "error": { - "code": 14, - "message": "Simulated Backend Error" - }, - "runBackoffTimer": true - } - }, { "watchStreamClose": { "error": { @@ -283,15 +264,6 @@ } } }, - { - "watchStreamClose": { - "error": { - "code": 14, - "message": "Simulated Backend Error" - }, - "runBackoffTimer": true - } - }, { "watchStreamClose": { "error": { @@ -318,7 +290,10 @@ "Queries revert to fromCache=true when offline.": { "describeName": "Offline:", "itName": "Queries revert to fromCache=true when offline.", - "tags": [], + "tags": [ + "no-android", + "no-ios" + ], "config": { "useGarbageCollection": true, "numClients": 1 @@ -422,15 +397,6 @@ } } }, - { - "watchStreamClose": { - "error": { - "code": 14, - "message": "Simulated Backend Error" - }, - "runBackoffTimer": true - } - }, { "watchStreamClose": { "error": { @@ -495,7 +461,10 @@ "Queries with limbo documents handle going offline.": { "describeName": "Offline:", "itName": "Queries with limbo documents handle going offline.", - "tags": [], + "tags": [ + "no-android", + "no-ios" + ], "config": { "useGarbageCollection": true, "numClients": 1 @@ -669,15 +638,6 @@ "runBackoffTimer": true } }, - { - "watchStreamClose": { - "error": { - "code": 14, - "message": "Simulated Backend Error" - }, - "runBackoffTimer": true - } - }, { "watchAck": [ 2 @@ -929,7 +889,10 @@ "New queries return immediately with fromCache=true when offline due to stream failures.": { "describeName": "Offline:", "itName": "New queries return immediately with fromCache=true when offline due to stream failures.", - "tags": [], + "tags": [ + "no-android", + "no-ios" + ], "config": { "useGarbageCollection": true, "numClients": 1 @@ -957,15 +920,6 @@ } } }, - { - "watchStreamClose": { - "error": { - "code": 14, - "message": "Simulated Backend Error" - }, - "runBackoffTimer": true - } - }, { "watchStreamClose": { "error": { diff --git a/Firestore/Example/Tests/SpecTests/json/persistence_spec_test.json b/Firestore/Example/Tests/SpecTests/json/persistence_spec_test.json index 8f9aea8abd3..f5d9415f7bd 100644 --- a/Firestore/Example/Tests/SpecTests/json/persistence_spec_test.json +++ b/Firestore/Example/Tests/SpecTests/json/persistence_spec_test.json @@ -2,7 +2,9 @@ "Local mutations are persisted and re-sent": { "describeName": "Persistence:", "itName": "Local mutations are persisted and re-sent", - "tags": [], + "tags": [ + "durable-persistence" + ], "config": { "useGarbageCollection": true, "numClients": 1 @@ -50,7 +52,9 @@ "Persisted local mutations are visible to listeners": { "describeName": "Persistence:", "itName": "Persisted local mutations are visible to listeners", - "tags": [], + "tags": [ + "durable-persistence" + ], "config": { "useGarbageCollection": true, "numClients": 1 @@ -136,7 +140,9 @@ "Remote documents are persisted": { "describeName": "Persistence:", "itName": "Remote documents are persisted", - "tags": [], + "tags": [ + "durable-persistence" + ], "config": { "useGarbageCollection": true, "numClients": 1 @@ -586,7 +592,9 @@ "Mutation Queue is persisted across uid switches (with restarts)": { "describeName": "Persistence:", "itName": "Mutation Queue is persisted across uid switches (with restarts)", - "tags": [], + "tags": [ + "durable-persistence" + ], "config": { "useGarbageCollection": true, "numClients": 1 diff --git a/Firestore/Example/Tests/SpecTests/json/remote_store_spec_test.json b/Firestore/Example/Tests/SpecTests/json/remote_store_spec_test.json index 0d700ab2832..ebe521b2bbe 100644 --- a/Firestore/Example/Tests/SpecTests/json/remote_store_spec_test.json +++ b/Firestore/Example/Tests/SpecTests/json/remote_store_spec_test.json @@ -482,7 +482,10 @@ "Cleans up watch state correctly": { "describeName": "Remote store:", "itName": "Cleans up watch state correctly", - "tags": [], + "tags": [ + "no-android", + "no-ios" + ], "config": { "useGarbageCollection": false, "numClients": 1 @@ -517,7 +520,19 @@ "message": "Simulated Backend Error" }, "runBackoffTimer": true - } + }, + "expect": [ + { + "query": { + "path": "collection", + "filters": [], + "orderBys": [] + }, + "errorCode": 0, + "fromCache": true, + "hasPendingWrites": false + } + ] }, { "watchAck": [ @@ -618,6 +633,18 @@ }, "runBackoffTimer": false }, + "expect": [ + { + "query": { + "path": "collection", + "filters": [], + "orderBys": [] + }, + "errorCode": 0, + "fromCache": true, + "hasPendingWrites": false + } + ], "stateExpect": { "activeTargets": {} } diff --git a/Firestore/Example/Tests/SpecTests/json/write_spec_test.json b/Firestore/Example/Tests/SpecTests/json/write_spec_test.json index 6305d56c9fa..252fb382d95 100644 --- a/Firestore/Example/Tests/SpecTests/json/write_spec_test.json +++ b/Firestore/Example/Tests/SpecTests/json/write_spec_test.json @@ -4028,7 +4028,7 @@ "describeName": "Writes:", "itName": "Held writes are released when there are no queries left.", "tags": [ - "no-lru" + "eager-gc" ], "comment": "This test expects a new target id for a new listen, but without eager gc, the same target id is reused", "config": { From e25ed1ea5e4334c785242fa7cd54604b95e2f933 Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Fri, 17 Aug 2018 16:21:35 -0700 Subject: [PATCH 2/2] Start restarting persistence in spec tests --- .../Tests/SpecTests/FSTLevelDBSpecTests.mm | 17 +++++++++--- .../Tests/SpecTests/FSTMemorySpecTests.mm | 8 ++++++ .../Example/Tests/SpecTests/FSTSpecTests.h | 6 ++++- .../Example/Tests/SpecTests/FSTSpecTests.mm | 27 +++++++++---------- .../SpecTests/FSTSyncEngineTestDriver.mm | 3 +++ 5 files changed, 42 insertions(+), 19 deletions(-) diff --git a/Firestore/Example/Tests/SpecTests/FSTLevelDBSpecTests.mm b/Firestore/Example/Tests/SpecTests/FSTLevelDBSpecTests.mm index 74072c40db3..7944f5a5ba1 100644 --- a/Firestore/Example/Tests/SpecTests/FSTLevelDBSpecTests.mm +++ b/Firestore/Example/Tests/SpecTests/FSTLevelDBSpecTests.mm @@ -17,10 +17,13 @@ #import "Firestore/Example/Tests/SpecTests/FSTSpecTests.h" #import "Firestore/Source/Local/FSTLevelDB.h" +#include "Firestore/core/src/firebase/firestore/util/path.h" #import "Firestore/Example/Tests/Local/FSTPersistenceTestHelpers.h" #import "Firestore/Example/Tests/SpecTests/FSTSyncEngineTestDriver.h" +using firebase::firestore::util::Path; + NS_ASSUME_NONNULL_BEGIN /** @@ -31,15 +34,23 @@ @interface FSTLevelDBSpecTests : FSTSpecTests @end -@implementation FSTLevelDBSpecTests +@implementation FSTLevelDBSpecTests { + Path _levelDbDir; +} + +- (void)setUpForSpecWithConfig:(NSDictionary *)config { + // Getting a new directory will ensure that it is empty. + _levelDbDir = [FSTPersistenceTestHelpers levelDBDir]; + [super setUpForSpecWithConfig:config]; +} /** Overrides -[FSTSpecTests persistence] */ - (id)persistenceWithGCEnabled:(__unused BOOL)GCEnabled { - return [FSTPersistenceTestHelpers levelDBPersistence]; + return [FSTPersistenceTestHelpers levelDBPersistenceWithDir:_levelDbDir]; } - (BOOL)shouldRunWithTags:(NSArray *)tags { - if ([tags containsObject:kNoLRUTag]) { + if ([tags containsObject:kEagerGC]) { return NO; } diff --git a/Firestore/Example/Tests/SpecTests/FSTMemorySpecTests.mm b/Firestore/Example/Tests/SpecTests/FSTMemorySpecTests.mm index 6508a63676a..840ae992c2f 100644 --- a/Firestore/Example/Tests/SpecTests/FSTMemorySpecTests.mm +++ b/Firestore/Example/Tests/SpecTests/FSTMemorySpecTests.mm @@ -42,6 +42,14 @@ @implementation FSTMemorySpecTests } } +- (BOOL)shouldRunWithTags:(NSArray *)tags { + if ([tags containsObject:kDurablePersistence]) { + return NO; + } + + return [super shouldRunWithTags:tags]; +} + @end NS_ASSUME_NONNULL_END diff --git a/Firestore/Example/Tests/SpecTests/FSTSpecTests.h b/Firestore/Example/Tests/SpecTests/FSTSpecTests.h index a6fd8b4c104..4dd028171eb 100644 --- a/Firestore/Example/Tests/SpecTests/FSTSpecTests.h +++ b/Firestore/Example/Tests/SpecTests/FSTSpecTests.h @@ -21,7 +21,8 @@ NS_ASSUME_NONNULL_BEGIN -extern NSString *const kNoLRUTag; +extern NSString *const kEagerGC; +extern NSString *const kDurablePersistence; /** * FSTSpecTests run a set of portable event specifications from JSON spec files against a @@ -43,6 +44,9 @@ extern NSString *const kNoLRUTag; /** Based on its tags, determine whether the test case should run. */ - (BOOL)shouldRunWithTags:(NSArray *)tags; +/** Do any necessary setup for a single spec test */ +- (void)setUpForSpecWithConfig:(NSDictionary *)config; + @end NS_ASSUME_NONNULL_END diff --git a/Firestore/Example/Tests/SpecTests/FSTSpecTests.mm b/Firestore/Example/Tests/SpecTests/FSTSpecTests.mm index f8ee63bd5db..b84fd2aab24 100644 --- a/Firestore/Example/Tests/SpecTests/FSTSpecTests.mm +++ b/Firestore/Example/Tests/SpecTests/FSTSpecTests.mm @@ -76,7 +76,9 @@ // if `kRunBenchmarkTests` is set to 'YES'. static NSString *const kBenchmarkTag = @"benchmark"; -NSString *const kNoLRUTag = @"no-lru"; +NSString *const kEagerGC = @"eager-gc"; + +NSString *const kDurablePersistence = @"durable-persistence"; static NSString *Describe(NSData *data) { return [[NSString alloc] initWithData:data encoding:NSUTF8StringEncoding]; @@ -85,11 +87,11 @@ @interface FSTSpecTests () @property(nonatomic, strong) FSTSyncEngineTestDriver *driver; -// Some config info for the currently running spec; used when restarting the driver (for doRestart). -@property(nonatomic, strong) id driverPersistence; @end -@implementation FSTSpecTests +@implementation FSTSpecTests { + BOOL _gcEnabled; +} - (id)persistenceWithGCEnabled:(BOOL)GCEnabled { @throw FSTAbstractMethodException(); // NOLINT @@ -107,20 +109,20 @@ - (BOOL)shouldRunWithTags:(NSArray *)tags { } - (void)setUpForSpecWithConfig:(NSDictionary *)config { - // Store persistence / GCEnabled so we can re-use it in doRestart. + // Store GCEnabled so we can re-use it in doRestart. NSNumber *GCEnabled = config[@"useGarbageCollection"]; + _gcEnabled = [GCEnabled boolValue]; NSNumber *numClients = config[@"numClients"]; if (numClients) { XCTAssertEqualObjects(numClients, @1, @"The iOS client does not support multi-client tests"); } - self.driverPersistence = [self persistenceWithGCEnabled:[GCEnabled boolValue]]; - self.driver = [[FSTSyncEngineTestDriver alloc] initWithPersistence:self.driverPersistence]; + id persistence = [self persistenceWithGCEnabled:_gcEnabled]; + self.driver = [[FSTSyncEngineTestDriver alloc] initWithPersistence:persistence]; [self.driver start]; } - (void)tearDownForSpec { [self.driver shutdown]; - [self.driverPersistence shutdown]; } /** @@ -410,13 +412,8 @@ - (void)doRestart { [self.driver shutdown]; - // NOTE: We intentionally don't shutdown / re-create driverPersistence, since we want to - // preserve the persisted state. This is a bit of a cheat since it means we're not exercising - // the initialization / start logic that would normally be hit, but simplifies the plumbing and - // allows us to run these tests against FSTMemoryPersistence as well (there would be no way to - // re-create FSTMemoryPersistence without losing all persisted state). - - self.driver = [[FSTSyncEngineTestDriver alloc] initWithPersistence:self.driverPersistence + id persistence = [self persistenceWithGCEnabled:_gcEnabled]; + self.driver = [[FSTSyncEngineTestDriver alloc] initWithPersistence:persistence initialUser:currentUser outstandingWrites:outstandingWrites]; [self.driver start]; diff --git a/Firestore/Example/Tests/SpecTests/FSTSyncEngineTestDriver.mm b/Firestore/Example/Tests/SpecTests/FSTSyncEngineTestDriver.mm index 379ad811b00..7107d6183f9 100644 --- a/Firestore/Example/Tests/SpecTests/FSTSyncEngineTestDriver.mm +++ b/Firestore/Example/Tests/SpecTests/FSTSyncEngineTestDriver.mm @@ -77,6 +77,7 @@ @interface FSTSyncEngineTestDriver () @property(nonatomic, strong, readonly) FSTLocalStore *localStore; @property(nonatomic, strong, readonly) FSTSyncEngine *syncEngine; @property(nonatomic, strong, readonly) FSTDispatchQueue *dispatchQueue; +@property(nonatomic, strong, readonly) id persistence; #pragma mark - Data structures for holding events sent by the watch stream. @@ -129,6 +130,7 @@ - (instancetype)initWithPersistence:(id)persistence dispatch_queue_t queue = dispatch_queue_create("sync_engine_test_driver", DISPATCH_QUEUE_SERIAL); _dispatchQueue = [FSTDispatchQueue queueWith:queue]; + _persistence = persistence; _localStore = [[FSTLocalStore alloc] initWithPersistence:persistence initialUser:initialUser]; _datastore = [[FSTMockDatastore alloc] initWithDatabaseInfo:&_databaseInfo workerDispatchQueue:_dispatchQueue @@ -203,6 +205,7 @@ - (void)validateUsage { - (void)shutdown { [self.dispatchQueue dispatchSync:^{ [self.remoteStore shutdown]; + [self.persistence shutdown]; }]; }