Skip to content

enable disabled passed integration test #2120

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

Merged
merged 2 commits into from
Nov 26, 2018
Merged

enable disabled passed integration test #2120

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b4381fa
enable disabled passed integration test
zxu123 Nov 26, 2018
15351dd
address comment
zxu123 Nov 26, 2018
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
14 changes: 5 additions & 9 deletions Firestore/Example/Tests/Integration/FSTTransactionTests.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,7 @@ @interface FSTTransactionTests : FSTIntegrationTestCase

@implementation FSTTransactionTests

// We currently require every document read to also be written.
// TODO(b/34879758): Re-enable this test once we fix it.
- (void)xtestGetDocuments {
- (void)testGetDocuments {
Copy link
Member

Choose a reason for hiding this comment

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

I'd recommend removing the TODO on the line above as it no longer makes sense (there's nothing to re-enable). There's already a TODO for this issue within the body (line 46)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

FIRFirestore *firestore = [self firestore];
FIRDocumentReference *doc = [[firestore collectionWithPath:@"spaces"] documentWithAutoID];
[self writeDocumentRef:doc data:@{@"foo" : @1, @"desc" : @"Stuff", @"owner" : @"Jonny"}];
Expand Down Expand Up @@ -334,9 +332,7 @@ - (void)testUpdateTransactionally {
XCTAssertEqualObjects(@"yes", snapshot[@"other"]);
}

// We currently require every document read to also be written.
// TODO(b/34879758): Re-enable this test once we fix it.
- (void)xtestHandleReadingOneDocAndWritingAnother {
- (void)testHandleReadingOneDocAndWritingAnother {
Copy link
Member

Choose a reason for hiding this comment

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

As above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

FIRFirestore *firestore = [self firestore];
FIRDocumentReference *doc1 = [[firestore collectionWithPath:@"counters"] documentWithAutoID];
FIRDocumentReference *doc2 = [[firestore collectionWithPath:@"counters"] documentWithAutoID];
Expand Down Expand Up @@ -423,9 +419,7 @@ - (void)testReadingADocTwiceWithDifferentVersions {
XCTAssertEqualObjects(@(1234.0), snapshot[@"count"]);
}

// We currently require every document read to also be written.
// TODO(b/34879758): Add this test back once we fix that.
- (void)xtestCannotHaveAGetWithoutMutations {
- (void)testCannotHaveAGetWithoutMutations {
Copy link
Member

Choose a reason for hiding this comment

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

Similar to as above, but slightly different. In this test, there's no todo in teh body. So I'd recommend removing the TODO comment (like the others) but add in a TODO after line 440, similar to the first test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

FIRFirestore *firestore = [self firestore];
FIRDocumentReference *doc = [[firestore collectionWithPath:@"foo"] documentWithAutoID];
[self writeDocumentRef:doc data:@{@"foo" : @"bar"}];
Expand All @@ -438,6 +432,8 @@ - (void)xtestCannotHaveAGetWithoutMutations {
return nil;
}
completion:^(id _Nullable result, NSError *_Nullable error) {
// We currently require every document read to also be written.
// TODO(b/34879758): Fix this check once we drop that requirement.
XCTAssertNotNil(error);
[expectation fulfill];
}];
Expand Down