Skip to content

Split GoogleUtilities from FirebaseCore #1370

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.

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 49 commits into from
Jul 6, 2018
Merged
Show file tree
Hide file tree
Changes from 48 commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
03676a5
Split FirebaseUtilities from FirebaseCore
paulb777 Jun 2, 2018
22f72f8
Utilities minimum version to 6.0
paulb777 Jun 2, 2018
d9c9a08
Rename FirebaseUtilities to GoogleUtilities
paulb777 Jun 21, 2018
958cde6
style
paulb777 Jun 21, 2018
c06da6c
WIP - restore FIRLogger as a wrapper to GULLogger - pass Core unit tests
paulb777 Jun 23, 2018
3c2c4a6
style
paulb777 Jun 23, 2018
001a21f
checkpoint
paulb777 Jun 23, 2018
94be7d2
temporary Environment wrapper
paulb777 Jun 23, 2018
9509e41
checkpoint
paulb777 Jun 26, 2018
49c1f64
WIP Checkpoint - Core and Auth unit tests
paulb777 Jun 26, 2018
ef909e2
WIP checkpoint - all Firebase unit tests
paulb777 Jun 26, 2018
253e4bf
style
paulb777 Jun 26, 2018
cb29d53
Restore Firestore
paulb777 Jun 26, 2018
23e830c
WIP Access GoogleUtilities for Firestore
paulb777 Jun 27, 2018
96c7363
WIP Access GoogleUtilities for Firestore
paulb777 Jun 27, 2018
ca7ca3e
renames
paulb777 Jun 27, 2018
ea4d3f2
Force GoogleUtilities to link with FirebaseCore in cmake
paulb777 Jun 27, 2018
ab81c54
style
paulb777 Jun 27, 2018
abf110c
Try again with GoogleUtilities
paulb777 Jun 28, 2018
f1bfad2
disable failing FIRLogger tests
paulb777 Jun 28, 2018
6562098
style
paulb777 Jun 28, 2018
bedc87a
fix typo
paulb777 Jun 28, 2018
0e3e504
one more loggertest disable
paulb777 Jun 28, 2018
ed43b69
Don't integrate Analytics into unit tests
paulb777 Jun 28, 2018
dc9e6f0
Remove wrappers that were only for Analytics linking
paulb777 Jun 28, 2018
fe777a9
Split Reachability from Networking - part 1
paulb777 Jun 28, 2018
3b83363
Use local GoogleUtilities in Firestore unit testing
paulb777 Jun 28, 2018
fd568e0
Replace GoogleToolboxForMac with a GoogleUtilities subspec
paulb777 Jun 28, 2018
11ed49f
pod deintegrate
paulb777 Jun 28, 2018
e8fd9e7
style and podspec fix
paulb777 Jun 28, 2018
e1555da
fix bad merge
paulb777 Jun 28, 2018
ce59ba2
macOS and tvOS Core tests now need to force category loading
paulb777 Jun 29, 2018
6d49931
Option on app not tests
paulb777 Jun 29, 2018
e022441
clean up
paulb777 Jun 29, 2018
970f8c3
podspec cleanup
paulb777 Jun 29, 2018
739e6c0
address review feedback
paulb777 Jun 30, 2018
a123f6e
Fix bad rebase merge
paulb777 Jun 30, 2018
9c8f282
Update GoogleUtilities version
paulb777 Jun 30, 2018
74d74bf
style
paulb777 Jun 30, 2018
4bc7166
podspec consistency
paulb777 Jun 30, 2018
640550b
Add missing Security framework for SecTrustEvaluate API call
paulb777 Jul 1, 2018
df4c242
Consistently use module reference for public/private headers
paulb777 Jul 1, 2018
bce3438
delete unnecessary import
paulb777 Jul 1, 2018
aa2f13f
consistency
paulb777 Jul 1, 2018
1592bf9
Add library dependency
paulb777 Jul 2, 2018
9c26db1
GoogleUtilities does not have to be static - let Podfile determine
paulb777 Jul 2, 2018
988aee0
Fix non-modular import error
paulb777 Jul 2, 2018
c2e9973
fix analytics tests
paulb777 Jul 3, 2018
aeb878b
review updates
paulb777 Jul 6, 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
38 changes: 24 additions & 14 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -52,50 +52,60 @@ jobs:
- stage: test
env:
- PROJECT=Firebase PLATFORM=iOS METHOD=pod-lib-lint
# Set ALT_SOURCES like the following to continue lint testing until release when Utilities
# or Core APIs change. GoogleUtilities.podspec and FirebaseCore.podspec should be
# manually pushed to a temporary Specs repo. See
# https://guides.cocoapods.org/making/private-cocoapods.
# ALT_SOURCES="[email protected]:paulb777/Specs.git,https://github.com/CocoaPods/Specs.git"
- ALT_SOURCES="--sources=https://github.com/paulb777/Specs.git,https://github.com/CocoaPods/Specs.git"
before_install:
- ./scripts/if_changed.sh ./scripts/install_prereqs.sh
script:
- ./scripts/if_changed.sh bundle exec pod lib lint FirebaseCore.podspec
- ./scripts/if_changed.sh bundle exec pod lib lint FirebaseAuth.podspec
- ./scripts/if_changed.sh bundle exec pod lib lint FirebaseDatabase.podspec
- ./scripts/if_changed.sh bundle exec pod lib lint FirebaseMessaging.podspec
- ./scripts/if_changed.sh bundle exec pod lib lint FirebaseStorage.podspec
- ./scripts/if_changed.sh bundle exec pod lib lint FirebaseFunctions.podspec
- ./scripts/if_changed.sh bundle exec pod lib lint GoogleUtilities.podspec
- ./scripts/if_changed.sh bundle exec pod lib lint FirebaseCore.podspec $ALT_SOURCES
- ./scripts/if_changed.sh bundle exec pod lib lint FirebaseAuth.podspec $ALT_SOURCES
- ./scripts/if_changed.sh bundle exec pod lib lint FirebaseDatabase.podspec $ALT_SOURCES
- ./scripts/if_changed.sh bundle exec pod lib lint FirebaseMessaging.podspec $ALT_SOURCES
- ./scripts/if_changed.sh bundle exec pod lib lint FirebaseStorage.podspec $ALT_SOURCES
- ./scripts/if_changed.sh bundle exec pod lib lint FirebaseFunctions.podspec $ALT_SOURCES

- stage: test
env:
- PROJECT=Firestore PLATFORM=iOS METHOD=pod-lib-lint
- ALT_SOURCES="--sources=https://github.com/paulb777/Specs.git,https://github.com/CocoaPods/Specs.git"
before_install:
- ./scripts/if_changed.sh ./scripts/install_prereqs.sh
script:
# Eliminate the one warning from BoringSSL when CocoaPods 1.6.0 is available.
# The travis_wait is necessary because the command takes more than 10 minutes.
- travis_wait ./scripts/if_changed.sh bundle exec pod lib lint FirebaseFirestore.podspec --allow-warnings --no-subspecs
- travis_wait ./scripts/if_changed.sh bundle exec pod lib lint FirebaseFirestore.podspec --allow-warnings --no-subspecs $ALT_SOURCES

# pod lib lint to check build and warnings for static library build - only on cron jobs
- stage: test
env:
- PROJECT=Firebase PLATFORM=iOS METHOD=pod-lib-lint
- ALT_SOURCES="--sources=https://github.com/paulb777/Specs.git,https://github.com/CocoaPods/Specs.git"
before_install:
- ./scripts/if_cron.sh ./scripts/install_prereqs.sh
script:
- ./scripts/if_cron.sh bundle exec pod lib lint FirebaseCore.podspec --use-libraries
- ./scripts/if_cron.sh bundle exec pod lib lint FirebaseAuth.podspec --use-libraries
- ./scripts/if_cron.sh bundle exec pod lib lint FirebaseDatabase.podspec --use-libraries
- ./scripts/if_cron.sh bundle exec pod lib lint FirebaseCore.podspec --use-libraries $ALT_SOURCES
- ./scripts/if_cron.sh bundle exec pod lib lint FirebaseAuth.podspec --use-libraries $ALT_SOURCES
- ./scripts/if_cron.sh bundle exec pod lib lint FirebaseDatabase.podspec --use-libraries $ALT_SOURCES
# The Protobuf dependency of FirebaseMessaging has warnings with --use-libraries
- ./scripts/if_cron.sh bundle exec pod lib lint FirebaseMessaging.podspec --use-libraries --allow-warnings
- ./scripts/if_cron.sh bundle exec pod lib lint FirebaseStorage.podspec --use-libraries
- ./scripts/if_cron.sh bundle exec pod lib lint FirebaseFunctions.podspec --use-libraries
- ./scripts/if_cron.sh bundle exec pod lib lint FirebaseMessaging.podspec --use-libraries --allow-warnings $ALT_SOURCES
- ./scripts/if_cron.sh bundle exec pod lib lint FirebaseStorage.podspec --use-libraries $ALT_SOURCES
- ./scripts/if_cron.sh bundle exec pod lib lint FirebaseFunctions.podspec --use-libraries $ALT_SOURCES

- stage: test
env:
- PROJECT=Firestore PLATFORM=iOS METHOD=pod-lib-lint
- ALT_SOURCES="--sources=https://github.com/paulb777/Specs.git,https://github.com/CocoaPods/Specs.git"
before_install:
- ./scripts/if_cron.sh ./scripts/install_prereqs.sh
script:
# TBD - non-portable path warnings
# The travis_wait is necessary because the command takes more than 10 minutes.
- travis_wait ./scripts/if_cron.sh bundle exec pod lib lint FirebaseFirestore.podspec --use-libraries --allow-warnings --no-subspecs
- travis_wait ./scripts/if_cron.sh bundle exec pod lib lint FirebaseFirestore.podspec --use-libraries --allow-warnings --no-subspecs $ALT_SOURCES

# Alternative platforms

Expand Down
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ enable_testing()
# These are ordered by a topological sort on DEPENDS attributes. This is
# required because CMake fails the build if you have a DEPENDS on a target that
# does not exist yet.
include(external/GoogleUtilities)
include(external/FirebaseCore)
include(external/googletest)
include(external/zlib)
Expand Down
12 changes: 6 additions & 6 deletions Example/Core/Tests/FIRAppEnvironmentUtilTest.m
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,17 @@
#import <Foundation/Foundation.h>
#import <XCTest/XCTest.h>

#import <FirebaseCore/FIRAppEnvironmentUtil.h>
#import <GoogleUtilities/GULAppEnvironmentUtil.h>
Copy link
Member

Choose a reason for hiding this comment

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

Should this eventually just be moved to a GoogleUtilities test suite?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. The tests will be factored out in a subsequent PR.


#import "FIRTestCase.h"

@interface FIRAppEnvironmentUtilTest : FIRTestCase
@interface GULAppEnvironmentUtilTest : FIRTestCase

@property(nonatomic) id processInfoMock;

@end

@implementation FIRAppEnvironmentUtilTest
@implementation GULAppEnvironmentUtilTest
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this test move along with the sources?

This comment applies both here and to the other tests in Core in this PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep. The tests will be factored out in a subsequent PR.


- (void)setUp {
[super setUp];
Expand All @@ -43,21 +43,21 @@ - (void)testSystemVersionInfoMajorOnly {
NSOperatingSystemVersion osTen = {.majorVersion = 10, .minorVersion = 0, .patchVersion = 0};
OCMStub([self.processInfoMock operatingSystemVersion]).andReturn(osTen);

XCTAssertTrue([[FIRAppEnvironmentUtil systemVersion] isEqualToString:@"10.0"]);
XCTAssertTrue([[GULAppEnvironmentUtil systemVersion] isEqualToString:@"10.0"]);
}

- (void)testSystemVersionInfoMajorMinor {
NSOperatingSystemVersion osTenTwo = {.majorVersion = 10, .minorVersion = 2, .patchVersion = 0};
OCMStub([self.processInfoMock operatingSystemVersion]).andReturn(osTenTwo);

XCTAssertTrue([[FIRAppEnvironmentUtil systemVersion] isEqualToString:@"10.2"]);
XCTAssertTrue([[GULAppEnvironmentUtil systemVersion] isEqualToString:@"10.2"]);
}

- (void)testSystemVersionInfoMajorMinorPatch {
NSOperatingSystemVersion osTenTwoOne = {.majorVersion = 10, .minorVersion = 2, .patchVersion = 1};
OCMStub([self.processInfoMock operatingSystemVersion]).andReturn(osTenTwoOne);

XCTAssertTrue([[FIRAppEnvironmentUtil systemVersion] isEqualToString:@"10.2.1"]);
XCTAssertTrue([[GULAppEnvironmentUtil systemVersion] isEqualToString:@"10.2.1"]);
}

@end
39 changes: 24 additions & 15 deletions Example/Core/Tests/FIRLoggerTest.m
Original file line number Diff line number Diff line change
Expand Up @@ -17,27 +17,30 @@

#import "FIRTestCase.h"

// TODO - FIRLoggerTest should be split into a separate FIRLoggerTest and GULLoggerTest.
// No test should include both includes.
#import <FirebaseCore/FIRLogger.h>
#import <GoogleUtilities/GULLogger.h>
Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't seem like anything should import both FIRLogger and GULLogger.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed. Adding a TODO comment to split FIRLoggerTest from GULLoggerTest.


#import <asl.h>

// The following constants are exposed from FIRLogger for unit tests.
extern NSString *const kFIRDisableDebugModeApplicationArgument;
extern NSString *const kFIREnableDebugModeApplicationArgument;

extern NSString *const kFIRPersistedDebugModeKey;
extern NSString *const kGULPersistedDebugModeKey;

extern const char *kFIRLoggerASLClientFacilityName;
extern const char *kGULLoggerASLClientFacilityName;

extern void FIRResetLogger(void);

extern void FIRSetLoggerUserDefaults(NSUserDefaults *defaults);

extern aslclient getFIRLoggerClient(void);
extern aslclient getGULLoggerClient(void);

extern dispatch_queue_t getFIRClientQueue(void);
extern dispatch_queue_t getGULClientQueue(void);

extern BOOL getFIRLoggerDebugMode(void);
extern BOOL getGULLoggerDebugMode(void);

static NSString *const kMessageCode = @"I-COR000001";

Expand Down Expand Up @@ -95,9 +98,11 @@ - (void)testInitializeASLForNonDebugMode {
FIRLogError(kFIRLoggerCore, kMessageCode, @"Some error.");

// Assert.
NSNumber *debugMode = [self.defaults objectForKey:kFIRPersistedDebugModeKey];
#if MAKE_THREAD_SAFE
NSNumber *debugMode = [self.defaults objectForKey:kGULPersistedDebugModeKey];
XCTAssertNil(debugMode);
XCTAssertFalse(getFIRLoggerDebugMode());
XCTAssertFalse(getGULLoggerDebugMode());
#endif

// Stop.
[processInfoMock stopMocking];
Expand All @@ -112,10 +117,12 @@ - (void)testInitializeASLForDebugModeWithArgument {
// Test.
FIRLogError(kFIRLoggerCore, kMessageCode, @"Some error.");

#ifdef MAKE_THREAD_SAFE
// Assert.
NSNumber *debugMode = [self.defaults objectForKey:kFIRPersistedDebugModeKey];
NSNumber *debugMode = [self.defaults objectForKey:kGULPersistedDebugModeKey];
XCTAssertTrue(debugMode.boolValue);
XCTAssertTrue(getFIRLoggerDebugMode());
XCTAssertTrue(getGULLoggerDebugMode());
#endif

// Stop.
[processInfoMock stopMocking];
Expand All @@ -124,15 +131,17 @@ - (void)testInitializeASLForDebugModeWithArgument {
- (void)testInitializeASLForDebugModeWithUserDefaults {
// Stub.
NSNumber *debugMode = @YES;
[self.defaults setBool:debugMode.boolValue forKey:kFIRPersistedDebugModeKey];
[self.defaults setBool:debugMode.boolValue forKey:kGULPersistedDebugModeKey];

// Test.
FIRLogError(kFIRLoggerCore, kMessageCode, @"Some error.");

// Assert.
debugMode = [self.defaults objectForKey:kFIRPersistedDebugModeKey];
debugMode = [self.defaults objectForKey:kGULPersistedDebugModeKey];
XCTAssertTrue(debugMode.boolValue);
XCTAssertTrue(getFIRLoggerDebugMode());
#if MAKE_THREAD_SAFE
XCTAssertTrue(getGULLoggerDebugMode());
#endif
}

- (void)testMessageCodeFormat {
Expand Down Expand Up @@ -240,7 +249,7 @@ - (BOOL)logExists {

- (void)drainFIRClientQueue {
dispatch_semaphore_t workerSemaphore = dispatch_semaphore_create(0);
dispatch_async(getFIRClientQueue(), ^{
dispatch_async(getGULClientQueue(), ^{
dispatch_semaphore_signal(workerSemaphore);
});
dispatch_semaphore_wait(workerSemaphore, DISPATCH_TIME_FOREVER);
Expand All @@ -250,8 +259,8 @@ - (BOOL)messageWasLogged:(NSString *)message {
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wdeprecated-declarations"
aslmsg query = asl_new(ASL_TYPE_QUERY);
asl_set_query(query, ASL_KEY_FACILITY, kFIRLoggerASLClientFacilityName, ASL_QUERY_OP_EQUAL);
aslresponse r = asl_search(getFIRLoggerClient(), query);
asl_set_query(query, ASL_KEY_FACILITY, kGULLoggerASLClientFacilityName, ASL_QUERY_OP_EQUAL);
aslresponse r = asl_search(getGULLoggerClient(), query);
asl_free(query);
aslmsg m;
const char *val;
Expand Down
6 changes: 3 additions & 3 deletions Example/Core/Tests/FIRMutableDictionaryTest.m
Original file line number Diff line number Diff line change
Expand Up @@ -14,22 +14,22 @@

#import "FIRTestCase.h"

#import <FirebaseCore/FIRMutableDictionary.h>
#import <GoogleUtilities/GULMutableDictionary.h>

const static NSString *const kKey = @"testKey1";
const static NSString *const kValue = @"testValue1";
const static NSString *const kKey2 = @"testKey2";
const static NSString *const kValue2 = @"testValue2";

@interface FIRMutableDictionaryTest : FIRTestCase
@property(nonatomic) FIRMutableDictionary *dictionary;
@property(nonatomic) GULMutableDictionary *dictionary;
@end

@implementation FIRMutableDictionaryTest

- (void)setUp {
[super setUp];
self.dictionary = [[FIRMutableDictionary alloc] init];
self.dictionary = [[GULMutableDictionary alloc] init];
}

- (void)tearDown {
Expand Down
Loading