Skip to content

Commit

Permalink
Fix Firestore tests for M22 (firebase#834)
Browse files Browse the repository at this point in the history 
* Add FIRFirestoreTests to the Firestore Xcode project

* Avoid waitForExpectations:timeout:

This API was added in Xcode 8.3, but we still build production releases
with Xcode 8.2. waitForExpectationsWithTimeout:handler: is available
from Xcode 7.2.

* Add AppForUnitTesting

Add a utility for constructing a Firebase App for testing.

* Handle the nil UID from FIRAuth

* Avoid running CMake tests twice

* Only build app_testing on Apple platforms

* Revise test.sh messages
  • Loading branch information
@wilhuff
wilhuff committed Feb 22, 2018
1 parent 935f3ca commit 4dc63f8
Show file tree
Hide file tree
Showing 15 changed files with 227 additions and 79 deletions.
20 changes: 19 additions & 1 deletion Firestore/Example/Firestore.xcodeproj/project.pbxproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
/* Begin PBXBuildFile section */
3B843E4C1F3A182900548890 /* remote_store_spec_test.json in Resources */ = {isa = PBXBuildFile; fileRef = 3B843E4A1F3930A400548890 /* remote_store_spec_test.json */; };
5436F32420008FAD006E51E3 /* string_printf_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 5436F32320008FAD006E51E3 /* string_printf_test.cc */; };
5467FB01203E5717009C9584 /* FIRFirestoreTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5467FAFF203E56F8009C9584 /* FIRFirestoreTests.mm */; };
5467FB08203E6A44009C9584 /* app_testing.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5467FB07203E6A44009C9584 /* app_testing.mm */; };
54740A571FC914BA00713A1A /* secure_random_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 54740A531FC913E500713A1A /* secure_random_test.cc */; };
54740A581FC914F000713A1A /* autoid_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 54740A521FC913E500713A1A /* autoid_test.cc */; };
54764FAF1FAA21B90085E60A /* FSTGoogleTestTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 54764FAE1FAA21B90085E60A /* FSTGoogleTestTests.mm */; };
Expand Down Expand Up @@ -216,6 +218,9 @@
42491D7DC8C8CD245CC22B93 /* Pods-SwiftBuildTest.debug.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-SwiftBuildTest.debug.xcconfig"; path = "Pods/Target Support Files/Pods-SwiftBuildTest/Pods-SwiftBuildTest.debug.xcconfig"; sourceTree = "<group>"; };
4EBC5F5ABE1FD097EFE5E224 /* Pods-Firestore_Example.release.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-Firestore_Example.release.xcconfig"; path = "Pods/Target Support Files/Pods-Firestore_Example/Pods-Firestore_Example.release.xcconfig"; sourceTree = "<group>"; };
5436F32320008FAD006E51E3 /* string_printf_test.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = string_printf_test.cc; path = ../../core/test/firebase/firestore/util/string_printf_test.cc; sourceTree = "<group>"; };
5467FAFF203E56F8009C9584 /* FIRFirestoreTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = FIRFirestoreTests.mm; sourceTree = "<group>"; };
5467FB06203E6A44009C9584 /* app_testing.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = app_testing.h; path = ../../core/test/firebase/firestore/testutil/app_testing.h; sourceTree = "<group>"; };
5467FB07203E6A44009C9584 /* app_testing.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = app_testing.mm; path = ../../core/test/firebase/firestore/testutil/app_testing.mm; sourceTree = "<group>"; };
54740A521FC913E500713A1A /* autoid_test.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = autoid_test.cc; path = ../../core/test/firebase/firestore/util/autoid_test.cc; sourceTree = "<group>"; };
54740A531FC913E500713A1A /* secure_random_test.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = secure_random_test.cc; path = ../../core/test/firebase/firestore/util/secure_random_test.cc; sourceTree = "<group>"; };
54764FAE1FAA21B90085E60A /* FSTGoogleTestTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = FSTGoogleTestTests.mm; path = GoogleTest/FSTGoogleTestTests.mm; sourceTree = "<group>"; };
Expand Down Expand Up @@ -429,6 +434,15 @@
/* End PBXFrameworksBuildPhase section */

/* Begin PBXGroup section */
5467FB05203E652F009C9584 /* testutil */ = {
isa = PBXGroup;
children = (
5467FB06203E6A44009C9584 /* app_testing.h */,
5467FB07203E6A44009C9584 /* app_testing.mm */,
);
name = testutil;
sourceTree = "<group>";
};
54740A561FC913EB00713A1A /* util */ = {
isa = PBXGroup;
children = (
Expand All @@ -452,6 +466,7 @@
AB380CF7201937B800D97691 /* core */,
54EB764B202277970088B8F3 /* immutable */,
AB356EF5200E9D1A0089B766 /* model */,
5467FB05203E652F009C9584 /* testutil */,
54740A561FC913EB00713A1A /* util */,
54764FAE1FAA21B90085E60A /* FSTGoogleTestTests.mm */,
AB7BAB332012B519001E0872 /* geo_point_test.cc */,
Expand Down Expand Up @@ -697,16 +712,17 @@
DE51B1831F0D48AC0013853F /* API */ = {
isa = PBXGroup;
children = (
B65D34A7203C99090076A5E1 /* FIRTimestampTest.m */,
5492E045202154AA00B64F25 /* FIRCollectionReferenceTests.mm */,
5492E049202154AA00B64F25 /* FIRDocumentReferenceTests.mm */,
5492E04B202154AA00B64F25 /* FIRDocumentSnapshotTests.mm */,
5492E04C202154AA00B64F25 /* FIRFieldPathTests.mm */,
5492E04A202154AA00B64F25 /* FIRFieldValueTests.mm */,
5467FAFF203E56F8009C9584 /* FIRFirestoreTests.mm */,
5492E048202154AA00B64F25 /* FIRGeoPointTests.mm */,
5492E04F202154AA00B64F25 /* FIRQuerySnapshotTests.mm */,
5492E046202154AA00B64F25 /* FIRQueryTests.mm */,
5492E04D202154AA00B64F25 /* FIRSnapshotMetadataTests.mm */,
B65D34A7203C99090076A5E1 /* FIRTimestampTest.m */,
5492E047202154AA00B64F25 /* FSTAPIHelpers.h */,
5492E04E202154AA00B64F25 /* FSTAPIHelpers.mm */,
);
Expand Down Expand Up @@ -1301,6 +1317,7 @@
54740A581FC914F000713A1A /* autoid_test.cc in Sources */,
548DB927200D590300E00ABC /* assert_test.cc in Sources */,
5492E0A62021552D00B64F25 /* FSTPersistenceTestHelpers.mm in Sources */,
5467FB01203E5717009C9584 /* FIRFirestoreTests.mm in Sources */,
5492E0A12021552D00B64F25 /* FSTMemoryLocalStoreTests.mm in Sources */,
5436F32420008FAD006E51E3 /* string_printf_test.cc in Sources */,
5492E067202154B900B64F25 /* FSTEventManagerTests.mm in Sources */,
Expand Down Expand Up @@ -1334,6 +1351,7 @@
5492E065202154B900B64F25 /* FSTViewTests.mm in Sources */,
5492E03C2021401F00B64F25 /* XCTestCase+Await.mm in Sources */,
B6152AD7202A53CB000E5744 /* document_key_test.cc in Sources */,
5467FB08203E6A44009C9584 /* app_testing.mm in Sources */,
54764FAF1FAA21B90085E60A /* FSTGoogleTestTests.mm in Sources */,
AB380D04201BC6E400D97691 /* ordered_code_test.cc in Sources */,
5492E03F2021401F00B64F25 /* FSTHelpers.mm in Sources */,
Expand Down
14 changes: 13 additions & 1 deletion Firestore/Example/Firestore.xcodeproj/xcshareddata/xcschemes/Firestore_Tests.xcscheme
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,11 @@
buildImplicitDependencies = "YES">
<BuildActionEntries>
<BuildActionEntry
buildForTesting = "YES"
buildForRunning = "YES"
buildForTesting = "YES">
buildForProfiling = "YES"
buildForArchiving = "YES"
buildForAnalyzing = "YES">
<BuildableReference
BuildableIdentifier = "primary"
BlueprintIdentifier = "6003F5AD195388D20070C39A"
Expand Down Expand Up @@ -51,6 +54,15 @@
debugDocumentVersioning = "YES"
debugServiceExtension = "internal"
allowLocationSimulation = "YES">
<MacroExpansion>
<BuildableReference
BuildableIdentifier = "primary"
BlueprintIdentifier = "6003F5AD195388D20070C39A"
BuildableName = "Firestore_Tests.xctest"
BlueprintName = "Firestore_Tests"
ReferencedContainer = "container:Firestore.xcodeproj">
</BuildableReference>
</MacroExpansion>
<AdditionalOptions>
</AdditionalOptions>
</LaunchAction>
Expand Down
21 changes: 12 additions & 9 deletions Firestore/Example/Tests/API/FIRFirestoreTests.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,21 @@

#import <XCTest/XCTest.h>

#include "Firestore/core/test/firebase/firestore/testutil/app_testing.h"

namespace testutil = firebase::firestore::testutil;

@interface FIRFirestoreTests : XCTestCase
@end

@implementation FIRFirestoreTests

- (void)testDeleteApp {
// Create a FIRApp for testing.
NSString *appName = @"custom_app_name";
FIROptions *options =
[[FIROptions alloc] initWithGoogleAppID:@"1:123:ios:123ab" GCMSenderID:@"gcm_sender_id"];
options.projectID = @"project_id";
[FIRApp configureWithName:appName options:options];

// Ensure the app is set appropriately.
FIRApp *app = [FIRApp appNamed:appName];
FIRApp *app = testutil::AppForUnitTesting();
NSString *appName = app.name;
FIROptions *options = app.options;

FIRFirestore *firestore = [FIRFirestore firestoreForApp:app];
XCTAssertEqualObjects(firestore.app, app);

Expand All @@ -56,7 +56,10 @@ - (void)testDeleteApp {
[defaultAppDeletedExpectation fulfill];
}];

[self waitForExpectations:@[ defaultAppDeletedExpectation ] timeout:2];
[self waitForExpectationsWithTimeout:2
handler:^(NSError *_Nullable error) {
XCTAssertNil(error);
}];
}

@end
8 changes: 3 additions & 5 deletions Firestore/Example/Tests/SpecTests/FSTSpecTests.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -332,12 +332,10 @@ - (void)doEnableNetwork {
}

- (void)doChangeUser:(id)UID {
if (UID == nil || [UID isEqual:[NSNull null]]) {
[self.driver changeUser:User::Unauthenticated()];
} else {
XCTAssert([UID isKindOfClass:[NSString class]]);
[self.driver changeUser:User(UID)];
if ([UID isEqual:[NSNull null]]) {
UID = nil;
}
[self.driver changeUser:User::FromUid(UID)];
}

- (void)doRestart {
Expand Down
6 changes: 3 additions & 3 deletions Firestore/Source/Auth/FSTCredentialsProvider.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ - (instancetype)initWithApp:(FIRApp *)app {
self = [super init];
if (self) {
_app = app;
_currentUser = User([self.app getUID]);
_currentUser = User::FromUid([self.app getUID]);
_userCounter = 0;

// Register for user changes so that we can internally track the current user.
Expand All @@ -84,8 +84,8 @@ - (instancetype)initWithApp:(FIRApp *)app {
return;
}

NSString *userID = userInfo[FIRAuthStateDidChangeInternalNotificationUIDKey];
User newUser = User(userID);
NSString *uid = userInfo[FIRAuthStateDidChangeInternalNotificationUIDKey];
User newUser = User::FromUid(uid);
if (newUser != self->_currentUser) {
self->_currentUser = newUser;
self.userCounter++;
Expand Down
1 change: 1 addition & 0 deletions Firestore/core/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ add_subdirectory(src/firebase/firestore/model)
add_subdirectory(src/firebase/firestore/remote)
add_subdirectory(src/firebase/firestore/util)

add_subdirectory(test/firebase/firestore/testutil)
add_subdirectory(test/firebase/firestore)
add_subdirectory(test/firebase/firestore/auth)
add_subdirectory(test/firebase/firestore/core)
Expand Down
5 changes: 3 additions & 2 deletions Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider_apple.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

#include <memory>
#include <mutex> // NOLINT(build/c++11)
#include <utility>

#include "Firestore/core/src/firebase/firestore/auth/credentials_provider.h"
#include "Firestore/core/src/firebase/firestore/auth/user.h"
Expand Down Expand Up @@ -76,8 +77,8 @@ class FirebaseCredentialsProvider : public CredentialsProvider {
* avoid races between notifications arriving and C++ object destruction.
*/
struct Contents {
Contents(FIRApp* app, const absl::string_view uid)
: app(app), current_user(uid), mutex() {
Contents(FIRApp* app, User&& user)
: app(app), current_user(std::move(user)), mutex() {
}

const FIRApp* app;
Expand Down
3 changes: 1 addition & 2 deletions Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider_apple.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,7 @@
namespace auth {

FirebaseCredentialsProvider::FirebaseCredentialsProvider(FIRApp* app)
: contents_(
std::make_shared<Contents>(app, util::MakeStringView([app getUID]))) {
: contents_(std::make_shared<Contents>(app, User::FromUid([app getUID]))) {
std::weak_ptr<Contents> weak_contents = contents_;

auth_listener_handle_ = [[NSNotificationCenter defaultCenter]
Expand Down
20 changes: 14 additions & 6 deletions Firestore/core/src/firebase/firestore/auth/user.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,6 @@ class User {
/** Construct an authenticated user with the given UID. */
explicit User(const absl::string_view uid);

#if defined(__OBJC__)
explicit User(NSString* uid)
: User(firebase::firestore::util::MakeStringView(uid)) {
}
#endif // defined(__OBJC__)

const std::string& uid() const {
return uid_;
}
Expand All @@ -62,6 +56,20 @@ class User {
/** Returns an unauthenticated instance. */
static const User& Unauthenticated();

#if defined(__OBJC__)
/**
* Returns an authenticated user if uid is non-nil, otherwise an
* unauthenticated user.
*/
static User FromUid(NSString* _Nullable uid) {
if (uid == nil) {
return Unauthenticated();
} else {
return User(util::MakeStringView(uid));
}
}
#endif // defined(__OBJC__)

User& operator=(const User& other) = default;

friend bool operator==(const User& lhs, const User& rhs);
Expand Down
1 change: 1 addition & 0 deletions Firestore/core/test/firebase/firestore/auth/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,5 +30,6 @@ if(APPLE)
firebase_credentials_provider_test.mm
DEPENDS
firebase_firestore_auth_apple
firebase_firestore_testutil
)
endif(APPLE)
71 changes: 28 additions & 43 deletions Firestore/core/test/firebase/firestore/auth/firebase_credentials_provider_test.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,74 +21,59 @@
#import <FirebaseCore/FIROptionsInternal.h>

#include "Firestore/core/src/firebase/firestore/util/string_apple.h"
#include "Firestore/core/test/firebase/firestore/testutil/app_testing.h"

#include "gtest/gtest.h"

namespace firebase {
namespace firestore {
namespace auth {

// TODO(zxu123): Make this an integration test and get infos from environment.
// Set a .plist file here to enable the test-case.
static NSString* const kPlist = @"";

class FirebaseCredentialsProviderTest : public ::testing::Test {
protected:
void SetUp() override {
app_ready_ = false;
if (![kPlist hasSuffix:@".plist"]) {
return;
}

static dispatch_once_t once_token;
dispatch_once(&once_token, ^{
FIROptions* options = [[FIROptions alloc] initWithContentsOfFile:kPlist];
[FIRApp configureWithOptions:options];
});
FIRApp* AppWithFakeUid(NSString* _Nullable uid) {
FIRApp* app = testutil::AppForUnitTesting();
app.getUIDImplementation = ^NSString* {
return uid;
};
return app;
}

// Set getUID implementation.
FIRApp* default_app = [FIRApp defaultApp];
default_app.getUIDImplementation = ^NSString* {
return @"I'm a fake uid.";
};
app_ready_ = true;
}
TEST(FirebaseCredentialsProviderTest, GetTokenUnauthenticated) {
FIRApp* app = AppWithFakeUid(nil);

bool app_ready_;
};
FirebaseCredentialsProvider credentials_provider(app);
credentials_provider.GetToken(
/*force_refresh=*/true, [](Token token, const absl::string_view error) {
EXPECT_EQ("", token.token());
const User& user = token.user();
EXPECT_EQ("", user.uid());
EXPECT_FALSE(user.is_authenticated());
EXPECT_EQ("", error) << error;
});
}

// Set kPlist above before enable.
TEST_F(FirebaseCredentialsProviderTest, GetToken) {
if (!app_ready_) {
return;
}
TEST(FirebaseCredentialsProviderTest, GetToken) {
FIRApp* app = AppWithFakeUid(@"fake uid");

FirebaseCredentialsProvider credentials_provider([FIRApp defaultApp]);
FirebaseCredentialsProvider credentials_provider(app);
credentials_provider.GetToken(
/*force_refresh=*/true, [](Token token, const absl::string_view error) {
EXPECT_EQ("", token.token());
const User& user = token.user();
EXPECT_EQ("I'm a fake uid.", user.uid());
EXPECT_EQ("fake uid", user.uid());
EXPECT_TRUE(user.is_authenticated());
EXPECT_EQ("", error) << error;
});
}

// Set kPlist above before enable.
TEST_F(FirebaseCredentialsProviderTest, SetListener) {
if (!app_ready_) {
return;
}
TEST(FirebaseCredentialsProviderTest, SetListener) {
FIRApp* app = AppWithFakeUid(@"fake uid");

FirebaseCredentialsProvider credentials_provider([FIRApp defaultApp]);
FirebaseCredentialsProvider credentials_provider(app);
credentials_provider.SetUserChangeListener([](User user) {
EXPECT_EQ("I'm a fake uid.", user.uid());
EXPECT_EQ("fake uid", user.uid());
EXPECT_TRUE(user.is_authenticated());
});

// TODO(wilhuff): We should wait for the above expectations to actually happen
// before continuing.

credentials_provider.SetUserChangeListener(nullptr);
}

Expand Down
Loading

0 comments on commit 4dc63f8

Please sign in to comment.