Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework sync config initialization #7857

Merged
merged 9 commits into from
Jul 27, 2022
Merged

Rework sync config initialization #7857

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
efe75b8
Switch to using node 16 for tests
tgoyne Jun 14, 2022
464231d
Rework sync config initialization
tgoyne Jun 17, 2022
960115a
Update SwiftUI sync tests for changes to the flexible sync API
tgoyne Jun 24, 2022
1a0f70e
Make RLMSyncManager.errorHandler atomic
tgoyne Jun 30, 2022
1d520f4
Lengthen some overly short timeouts in tests
tgoyne Jun 30, 2022
be9bcae
Fix a test which did not test what it intended to test
tgoyne Jul 1, 2022
9fad9d4
Add some missing test cleanup
tgoyne Jul 1, 2022
02dce57
Reduce the server polling period further and disable some unused things
tgoyne Jul 6, 2022
9e204e6
Test both flexible and partition-based sync Realm init
tgoyne Jul 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
x.y.z Release notes (yyyy-MM-dd)
=============================================================
### Enhancements
* None.
* Greatly improve the performance of obtaining cached Realm instances in Swift
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this enhancement coming from a user issue, could be valuable to tag this here?

when using a sync configuration.

### Fixed
* Add missing `initialSubscription` and `rerunOnOpen` to copyWithZone method on `RLMRealmConfiguration`. This resulted in incorrect values when using `RLMRealmConfiguration.defaultConfiguration`.
Expand Down
10 changes: 5 additions & 5 deletions Realm/ObjectServerTests/RLMFlexibleSyncServerTests.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -953,7 +953,7 @@ - (void)testFlexibleSyncInitialSubscriptionAwait {
CHECK_COUNT(11, Person, realm);
[ex fulfill];
}];
[self waitForExpectationsWithTimeout:10.0 handler:nil];
[self waitForExpectationsWithTimeout:30.0 handler:nil];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason for extending the timeout times in most of the server tests and the app creation?, isn't this enhancement supposed to make opening the realm faster

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests have been failing pretty consistently with timeouts due to the server being slow. All of the timeouts I bumped were ones which failed at least occasionally when using Xcode's "run tests repeatably until failure".

}

- (void)testFlexibleSyncInitialSubscriptionDoNotRerunOnOpen {
Expand Down Expand Up @@ -1007,7 +1007,7 @@ - (void)testFlexibleSyncInitialSubscriptionRerunOnOpen {
CHECK_COUNT(11, Person, realm);
[ex fulfill];
}];
[self waitForExpectationsWithTimeout:10.0 handler:nil];
[self waitForExpectationsWithTimeout:30.0 handler:nil];
XCTAssertEqual(openCount, 1);

__block RLMRealm *realm;
Expand All @@ -1021,7 +1021,7 @@ - (void)testFlexibleSyncInitialSubscriptionRerunOnOpen {
CHECK_COUNT(16, Person, realm);
[ex2 fulfill];
}];
[self waitForExpectationsWithTimeout:10.0 handler:nil];
[self waitForExpectationsWithTimeout:30.0 handler:nil];
XCTAssertEqual(openCount, 2);

[self dispatchAsyncAndWait:^{
Expand Down Expand Up @@ -1072,7 +1072,7 @@ - (void)testFlexibleSyncInitialOnConnectionTimeout {
XCTAssertNil(realm);
[ex fulfill];
}];
[self waitForExpectationsWithTimeout:10.0 handler:nil];
[self waitForExpectationsWithTimeout:30.0 handler:nil];

[proxy stop];
}
Expand All @@ -1095,6 +1095,6 @@ - (void)testFlexibleSyncInitialSubscriptionThrowsError {

[ex fulfill];
}];
[self waitForExpectationsWithTimeout:10.0 handler:nil];
[self waitForExpectationsWithTimeout:30.0 handler:nil];
}
@end
29 changes: 18 additions & 11 deletions Realm/ObjectServerTests/RLMObjectServerTests.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ - (void)testLogoutSpecificUser {
[expectation fulfill];
}];

[self waitForExpectationsWithTimeout:10.0 handler:nil];
[self waitForExpectationsWithTimeout:30.0 handler:nil];
}

- (void)testSwitchUser {
Expand Down Expand Up @@ -205,14 +205,14 @@ - (void)testDeviceRegistration {
XCTAssertNil(error);
[expectation fulfill];
}];
[self waitForExpectationsWithTimeout:10.0 handler:nil];
[self waitForExpectationsWithTimeout:30.0 handler:nil];

expectation = [self expectationWithDescription:@"should deregister device"];
[client deregisterDeviceForUser:self.app.currentUser completion:^(NSError *error) {
XCTAssertNil(error);
[expectation fulfill];
}];
[self waitForExpectationsWithTimeout:10.0 handler:nil];
[self waitForExpectationsWithTimeout:30.0 handler:nil];
}

// FIXME: Reenable once possible underlying race condition is understood
Expand Down Expand Up @@ -576,7 +576,7 @@ - (void)testSyncErrorHandlerErrorDomain {
encryptionKey:nil
stopPolicy:RLMSyncStopPolicyAfterChangesUploaded];

[self waitForExpectationsWithTimeout:10.0 handler:nil];
[self waitForExpectationsWithTimeout:30.0 handler:nil];
}

#pragma mark - User Profile
Expand Down Expand Up @@ -1432,7 +1432,7 @@ - (void)testClientReset {
[ex fulfill];
};
[user simulateClientResetErrorForSession:@"realm_id"];
[self waitForExpectationsWithTimeout:10 handler:nil];
[self waitForExpectationsWithTimeout:30 handler:nil];
XCTAssertNotNil(theError);
XCTAssertTrue(theError.code == RLMSyncErrorClientResetError);
NSString *pathValue = [theError rlmSync_clientResetBackedUpRealmPath];
Expand All @@ -1457,7 +1457,7 @@ - (void)testClientResetManualInitiation {
[ex fulfill];
};
[user simulateClientResetErrorForSession:partitionValue];
[self waitForExpectationsWithTimeout:10 handler:nil];
[self waitForExpectationsWithTimeout:30 handler:nil];
XCTAssertNotNil(theError);
}
// At this point the Realm should be invalidated and client reset should be possible.
Expand Down Expand Up @@ -1550,7 +1550,7 @@ - (void)testStreamingDownloadNotifier {
}];
// Wait for the child process to upload everything.
RLMRunChildAndWait();
[self waitForExpectationsWithTimeout:10.0 handler:nil];
[self waitForExpectationsWithTimeout:30.0 handler:nil];
[token invalidate];
// The notifier should have been called at least twice: once at the beginning and at least once
// to report progress.
Expand Down Expand Up @@ -1592,7 +1592,7 @@ - (void)testStreamingUploadNotifier {
}
[realm commitWriteTransaction];
// Wait for upload to begin and finish
[self waitForExpectationsWithTimeout:10.0 handler:nil];
[self waitForExpectationsWithTimeout:30.0 handler:nil];
[token invalidate];
// The notifier should have been called at least twice: once at the beginning and at least once
// to report progress.
Expand Down Expand Up @@ -1633,7 +1633,7 @@ - (void)testDownloadRealm {
return 0;
};
XCTAssertNil(RLMGetAnyCachedRealmForPath(c.pathOnDisk.UTF8String));
[self waitForExpectationsWithTimeout:10.0 handler:nil];
[self waitForExpectationsWithTimeout:30.0 handler:nil];
XCTAssertGreaterThan(fileSize(c.pathOnDisk), 0U);
XCTAssertNil(RLMGetAnyCachedRealmForPath(c.pathOnDisk.UTF8String));
}
Expand Down Expand Up @@ -1799,7 +1799,7 @@ - (void)testAsyncOpenConnectionTimeout {
XCTAssertNil(realm);
[ex fulfill];
}];
[self waitForExpectationsWithTimeout:10.0 handler:nil];
[self waitForExpectationsWithTimeout:30.0 handler:nil];

// Delay below the timeout should work
proxy.delay = 0.5;
Expand All @@ -1812,7 +1812,7 @@ - (void)testAsyncOpenConnectionTimeout {
XCTAssertNil(error);
[ex fulfill];
}];
[self waitForExpectationsWithTimeout:10.0 handler:nil];
[self waitForExpectationsWithTimeout:30.0 handler:nil];

[proxy stop];
}
Expand Down Expand Up @@ -2624,6 +2624,13 @@ - (void)testWatchWithMatchFilterAsync {

- (NSArray<RLMObjectId *> *)insertDogDocuments:(RLMMongoCollection *)collection {
__block NSArray<RLMObjectId *> *objectIds;
XCTestExpectation *ex = [self expectationWithDescription:@"delete existing documents"];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aren't we cleaning all documents on every tearDown, why duplicating it here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, we don't delete anything on tearDown. The sync tests use a separate partition for each test so that they don't see each others data, but these tests are accessing the collection directly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does deleting all documents before inserting obscure what this function is intended for? Should the calling function just call something like deleteAllDogDocuments and then call insertDogDocuments?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We never want to call them separately, but I guess the name should be prepareDogDocuments or something. The thing being fixed here is that without deleting old documents the tests only work when run in a specific order and break when run in other orders due to the missing cleanup.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, lets change the name to avoid confusion in the future

[collection deleteManyDocumentsWhere:@{} completion:^(NSInteger, NSError *error) {
XCTAssertNil(error);
[ex fulfill];
}];
[self waitForExpectations:@[ex] timeout:60.0];

XCTestExpectation *insertManyExpectation = [self expectationWithDescription:@"should insert documents"];
[collection insertManyDocuments:@[
@{@"name": @"fido", @"breed": @"cane corso"},
Expand Down
16 changes: 8 additions & 8 deletions Realm/ObjectServerTests/RLMSyncTestCase.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ - (RLMCredentials *)basicCredentialsWithName:(NSString *)name register:(BOOL)sho
XCTAssertNil(error);
[expectation fulfill];
}];
[self waitForExpectationsWithTimeout:4.0 handler:nil];
[self waitForExpectationsWithTimeout:20.0 handler:nil];
}
return [RLMCredentials credentialsWithEmail:name
password:@"password"];
Expand Down Expand Up @@ -209,7 +209,7 @@ - (RLMRealm *)asyncOpenRealmWithConfiguration:(RLMRealmConfiguration *)config {
r = realm;
[ex fulfill];
}];
[self waitForExpectationsWithTimeout:10.0 handler:nil];
[self waitForExpectationsWithTimeout:30.0 handler:nil];
// Ensure that the block does not retain the Realm, as it may not be dealloced
// immediately and so would extend the lifetime of the Realm an inconsistent amount
auto realm = r;
Expand All @@ -228,7 +228,7 @@ - (NSError *)asyncOpenErrorWithConfiguration:(RLMRealmConfiguration *)config {
error = err;
[ex fulfill];
}];
[self waitForExpectationsWithTimeout:10.0 handler:nil];
[self waitForExpectationsWithTimeout:30.0 handler:nil];
return error;
}

Expand Down Expand Up @@ -267,7 +267,7 @@ - (RLMUser *)logInUserForCredentials:(RLMCredentials *)credentials app:(RLMApp *
user = u;
[expectation fulfill];
}];
[self waitForExpectations:@[expectation] timeout:4.0];
[self waitForExpectations:@[expectation] timeout:20.0];
XCTAssertTrue(user.state == RLMUserStateLoggedIn, @"User should have been valid, but wasn't");
return user;
}
Expand All @@ -278,7 +278,7 @@ - (void)logOutUser:(RLMUser *)user {
XCTAssertNil(error);
[expectation fulfill];
}];
[self waitForExpectations:@[expectation] timeout:4.0];
[self waitForExpectations:@[expectation] timeout:20.0];
XCTAssertTrue(user.state == RLMUserStateLoggedOut, @"User should have been logged out, but wasn't");
}

Expand Down Expand Up @@ -359,7 +359,7 @@ - (void)waitForDownloadsForUser:(RLMUser *)user
XCTFail(@"Download waiter did not queue; session was invalid or errored out.");
return;
}
[self waitForExpectations:@[ex] timeout:20.0];
[self waitForExpectations:@[ex] timeout:60.0];
if (error) {
*error = theError;
}
Expand All @@ -378,7 +378,7 @@ - (void)waitForUploadsForRealm:(RLMRealm *)realm error:(NSError **)error {
XCTFail(@"Upload waiter did not queue; session was invalid or errored out.");
return;
}
[self waitForExpectations:@[ex] timeout:20.0];
[self waitForExpectations:@[ex] timeout:60.0];
if (error)
*error = completionError;
}
Expand All @@ -396,7 +396,7 @@ - (void)waitForDownloadsForRealm:(RLMRealm *)realm error:(NSError **)error {
XCTFail(@"Download waiter did not queue; session was invalid or errored out.");
return;
}
[self waitForExpectations:@[ex] timeout:20.0];
[self waitForExpectations:@[ex] timeout:60.0];
if (error) {
*error = completionError;
}
Expand Down
2 changes: 1 addition & 1 deletion Realm/ObjectServerTests/RealmServer.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ class Admin {
result = $0
group.leave()
}
guard case .success = group.wait(timeout: .now() + 5) else {
guard case .success = group.wait(timeout: .now() + 30) else {
return .failure(URLError(.badServerResponse))
}
return result
Expand Down
Loading