Firestore project fixes (#1301)

* Move GoogleTest.podspec out of the Tests tree
* Rename GoogleTests to CoreTests
* Move CoreTests to the top-level
* Move FSTGoogleTestTests.mm into core/test

4 files changed, 118 insertions, 85 deletions

diff --git a/Firestore/Example/Firestore.xcodeproj/project.pbxproj b/Firestore/Example/Firestore.xcodeproj/project.pbxproj
index 7ab01c4..e780b6c 100644
--- a/Firestore/Example/Firestore.xcodeproj/project.pbxproj
+++ b/Firestore/Example/Firestore.xcodeproj/project.pbxproj
@@ -267,7 +267,7 @@
 		546854A820A36867004BDBD5 /* datastore_test.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = datastore_test.cc; sourceTree = "<group>"; };
 		54740A521FC913E500713A1A /* autoid_test.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = autoid_test.cc; sourceTree = "<group>"; };
 		54740A531FC913E500713A1A /* secure_random_test.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = secure_random_test.cc; sourceTree = "<group>"; };
-		54764FAE1FAA21B90085E60A /* FSTGoogleTestTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = FSTGoogleTestTests.mm; path = ../../../../Example/Tests/GoogleTest/FSTGoogleTestTests.mm; sourceTree = "<group>"; };
+		54764FAE1FAA21B90085E60A /* FSTGoogleTestTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = FSTGoogleTestTests.mm; sourceTree = "<group>"; };
 		548DB926200D590300E00ABC /* assert_test.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = assert_test.cc; sourceTree = "<group>"; };
 		548DB928200D59F600E00ABC /* comparison_test.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = comparison_test.cc; sourceTree = "<group>"; };
 		5491BC711FB44593008B3588 /* FSTIntegrationTestCase.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = FSTIntegrationTestCase.mm; sourceTree = "<group>"; };
@@ -569,7 +569,7 @@
 			path = util;
 			sourceTree = "<group>";
 		};
-		54764FAC1FAA0C390085E60A /* GoogleTests */ = {
+		54764FAC1FAA0C390085E60A /* CoreTests */ = {
 			isa = PBXGroup;
 			children = (
 				AB38D9312023962A000A432D /* auth */,
@@ -584,8 +584,8 @@
 				AB7BAB332012B519001E0872 /* geo_point_test.cc */,
 				ABF6506B201131F8005F2C74 /* timestamp_test.cc */,
 			);
-			name = GoogleTests;
-			path = ../../core/test/firebase/firestore;
+			name = CoreTests;
+			path = ../core/test/firebase/firestore;
 			sourceTree = "<group>";
 		};
 		5495EB012040E90200EBA509 /* Codable */ = {
@@ -632,6 +632,7 @@
 				543B4F0520A91E4B001F506D /* App */,
 				60FF7A9C1954A5C5007DD14C /* Podspec Metadata */,
 				6003F5B5195388D20070C39A /* Tests */,
+				54764FAC1FAA0C390085E60A /* CoreTests */,
 				54C9EDF22040E16300A969CD /* SwiftTests */,
 				DE0761E51F2FE611003233AF /* SwiftBuildTest */,
 				6003F58C195388D20070C39A /* Frameworks */,
@@ -698,7 +699,6 @@
 			children = (
 				DE51B1831F0D48AC0013853F /* API */,
 				DE51B1A81F0D48AC0013853F /* Core */,
-				54764FAC1FAA0C390085E60A /* GoogleTests */,
 				DE2EF06E1F3D07D7003D0CDC /* Immutable */,
 				DE51B1BB1F0D48AC0013853F /* Integration */,
 				DE51B1621F0D48AC0013853F /* Local */,
diff --git a/Firestore/Example/Tests/GoogleTest/GoogleTest.podspec b/Firestore/Example/GoogleTest.podspec
index 0ecba3d..0ecba3d 100644
--- a/Firestore/Example/Tests/GoogleTest/GoogleTest.podspec
+++ b/Firestore/Example/GoogleTest.podspec
diff --git a/Firestore/Example/Podfile b/Firestore/Example/Podfile
index 7844066..26af4cd 100644
--- a/Firestore/Example/Podfile
+++ b/Firestore/Example/Podfile
@@ -21,7 +21,7 @@ target 'Firestore_Example_iOS' do
 
     pod 'leveldb-library'
     pod 'OCMock'
-    pod 'GoogleTest', :podspec => 'Tests/GoogleTest/GoogleTest.podspec'
+    pod 'GoogleTest', :podspec => 'GoogleTest.podspec'
   end
 
   target 'Firestore_IntegrationTests_iOS' do
diff --git a/Firestore/Example/Tests/GoogleTest/FSTGoogleTestTests.mm b/Firestore/core/test/firebase/firestore/FSTGoogleTestTests.mm
index ec14880..c87949a 100644
--- a/Firestore/Example/Tests/GoogleTest/FSTGoogleTestTests.mm
+++ b/Firestore/core/test/firebase/firestore/FSTGoogleTestTests.mm
@@ -22,53 +22,60 @@
 #include "gtest/gtest.h"
 
 /**
- * An XCTest test case that finds C++ test cases written in the GoogleTest framework, runs them, and
- * reports the results back to Xcode. This allows tests written in C++ that don't rely on XCTest to
- * coexist in this project.
+ * An XCTest test case that finds C++ test cases written in the GoogleTest
+ * framework, runs them, and reports the results back to Xcode. This allows
+ * tests written in C++ that don't rely on XCTest to coexist in this project.
  *
- * As an extra feature, you can run all C++ tests by focusing on the GoogleTests class.
+ * As an extra feature, you can run all C++ tests by focusing on the GoogleTests
+ * class.
  *
- * Each GoogleTest TestCase is mapped to a dynamically generated XCTestCase class. Each GoogleTest
- * TEST() is mapped to a test method on that XCTestCase.
+ * Each GoogleTest TestCase is mapped to a dynamically generated XCTestCase
+ * class. Each GoogleTest TEST() is mapped to a test method on that XCTestCase.
  */
 @interface GoogleTests : XCTestCase
 @end
 
 namespace {
 
-// A testing::TestCase named "Foo" corresponds to an XCTestCase named "FooTests".
+// A testing::TestCase named "Foo" corresponds to an XCTestCase named
+// "FooTests".
 NSString *const kTestCaseSuffix = @"Tests";
 
 // A testing::TestInfo named "Foo" corresponds to test method named "testFoo".
 NSString *const kTestMethodPrefix = @"test";
 
-// A map of keys created by TestInfoKey to the corresponding testing::TestInfo (wrapped in an
-// NSValue). The generated XCTestCase classes are discovered and instantiated by XCTest so this is
-// the only means of plumbing per-test-method state into these methods.
+// A map of keys created by TestInfoKey to the corresponding testing::TestInfo
+// (wrapped in an NSValue). The generated XCTestCase classes are discovered and
+// instantiated by XCTest so this is the only means of plumbing per-test-method
+// state into these methods.
 NSDictionary<NSString *, NSValue *> *testInfosByKey;
 
-// If the user focuses on GoogleTests itself, this means force all C++ tests to run.
+// If the user focuses on GoogleTests itself, this means force all C++ tests to
+// run.
 BOOL forceAllTests = NO;
 
 /**
- * Loads this XCTest runner's configuration file and figures out which tests to run based on the
- * contents of that configuration file.
+ * Loads this XCTest runner's configuration file and figures out which tests to
+ * run based on the contents of that configuration file.
  *
- * @return the set of tests to run, or nil if the user asked for all tests or if there's any
- *     problem loading or parsing the configuration.
+ * @return the set of tests to run, or nil if the user asked for all tests or if
+ * there's any problem loading or parsing the configuration.
  */
 NSSet<NSString *> *_Nullable LoadXCTestConfigurationTestsToRun() {
-  // Xcode invokes the test runner with an XCTestConfigurationFilePath environment variable set to
-  // the path of a configuration file containing, among other things, the set of tests to run. The
-  // configuration file deserializes to a non-public XCTestConfiguration class.
+  // Xcode invokes the test runner with an XCTestConfigurationFilePath
+  // environment variable set to the path of a configuration file containing,
+  // among other things, the set of tests to run. The configuration file
+  // deserializes to a non-public XCTestConfiguration class.
   //
-  // This loads that file and then reflectively pulls out the testsToRun set. Just in case any of
-  // these private details should change in the future and something should fail here, the mechanism
-  // complains but fails open. This way the worst that can happen is that users end up running more
-  // tests than they intend, but we never accidentally show a green run that wasn't.
+  // This loads that file and then reflectively pulls out the testsToRun set.
+  // Just in case any of these private details should change in the future and
+  // something should fail here, the mechanism complains but fails open. This
+  // way the worst that can happen is that users end up running more tests than
+  // they intend, but we never accidentally show a green run that wasn't.
   static NSString *const configEnvVar = @"XCTestConfigurationFilePath";
 
-  NSDictionary<NSString *, NSString *> *env = [[NSProcessInfo processInfo] environment];
+  NSDictionary<NSString *, NSString *> *env =
+      [[NSProcessInfo processInfo] environment];
   NSString *filePath = [env objectForKey:configEnvVar];
   if (!filePath) {
     NSLog(@"Missing %@ environment variable; assuming all tests", configEnvVar);
@@ -77,27 +84,32 @@ NSSet<NSString *> *_Nullable LoadXCTestConfigurationTestsToRun() {
 
   id config = [NSKeyedUnarchiver unarchiveObjectWithFile:filePath];
   if (!config) {
-    NSLog(@"Failed to load any configuaration from %@=%@", configEnvVar, filePath);
+    NSLog(@"Failed to load any configuaration from %@=%@", configEnvVar,
+          filePath);
     return nil;
   }
 
   SEL testsToRunSelector = NSSelectorFromString(@"testsToRun");
   if (![config respondsToSelector:testsToRunSelector]) {
-    NSLog(@"Invalid configuaration from %@=%@: missing testsToRun", configEnvVar, filePath);
+    NSLog(@"Invalid configuaration from %@=%@: missing testsToRun",
+          configEnvVar, filePath);
     return nil;
   }
 
-  // Invoke the testsToRun selector safely. This indirection is required because just calling
-  // -performSelector: fails to properly retain the NSSet under ARC.
+  // Invoke the testsToRun selector safely. This indirection is required because
+  // just calling -performSelector: fails to properly retain the NSSet under
+  // ARC.
   typedef NSSet<NSString *> *(*TestsToRunFunction)(id, SEL);
   IMP testsToRunMethod = [config methodForSelector:testsToRunSelector];
-  auto testsToRunFunction = reinterpret_cast<TestsToRunFunction>(testsToRunMethod);
+  auto testsToRunFunction =
+      reinterpret_cast<TestsToRunFunction>(testsToRunMethod);
   return testsToRunFunction(config, testsToRunSelector);
 }
 
 /**
- * Creates a GoogleTest filter specification, suitable for passing to the --gtest_filter flag,
- * that limits GoogleTest to running the same set of tests that Xcode requested.
+ * Creates a GoogleTest filter specification, suitable for passing to the
+ * --gtest_filter flag, that limits GoogleTest to running the same set of tests
+ * that Xcode requested.
  *
  * Each member of the testsToRun set is mapped as follows:
  *
@@ -106,7 +118,8 @@ NSSet<NSString *> *_Nullable LoadXCTestConfigurationTestsToRun() {
  *
  * These members are then joined with a ":" as googletest requires.
  *
- * @see https://github.com/google/googletest/blob/master/googletest/docs/AdvancedGuide.md
+ * @see
+ * https://github.com/google/googletest/blob/master/googletest/docs/AdvancedGuide.md
  */
 NSString *CreateTestFiltersFromTestsToRun(NSSet<NSString *> *testsToRun) {
   NSMutableString *result = [[NSMutableString alloc] init];
@@ -117,7 +130,8 @@ NSString *CreateTestFiltersFromTestsToRun(NSSet<NSString *> *testsToRun) {
     if (parts.count > 0) {
       NSString *className = parts[0];
       if ([className hasSuffix:kTestCaseSuffix]) {
-        gtestCaseName = [className substringToIndex:className.length - kTestCaseSuffix.length];
+        gtestCaseName = [className
+            substringToIndex:className.length - kTestCaseSuffix.length];
       }
     }
 
@@ -125,7 +139,8 @@ NSString *CreateTestFiltersFromTestsToRun(NSSet<NSString *> *testsToRun) {
     if (parts.count > 1) {
       NSString *methodName = parts[1];
       if ([methodName hasPrefix:kTestMethodPrefix]) {
-        gtestMethodName = [methodName substringFromIndex:kTestMethodPrefix.length];
+        gtestMethodName =
+            [methodName substringFromIndex:kTestMethodPrefix.length];
       }
     }
 
@@ -142,9 +157,11 @@ NSString *CreateTestFiltersFromTestsToRun(NSSet<NSString *> *testsToRun) {
   return result;
 }
 
-/** Returns the name of the selector for the test method representing this specific test. */
+/** Returns the name of the selector for the test method representing this
+ * specific test. */
 NSString *SelectorNameForTestInfo(const testing::TestInfo *testInfo) {
-  return [NSString stringWithFormat:@"%@%s", kTestMethodPrefix, testInfo->name()];
+  return
+      [NSString stringWithFormat:@"%@%s", kTestMethodPrefix, testInfo->name()];
 }
 
 /** Returns the name of the class representing the given testing::TestCase. */
@@ -153,21 +170,22 @@ NSString *ClassNameForTestCase(const testing::TestCase *testCase) {
 }
 
 /**
- * Returns a key name for the testInfosByKey dictionary. Each (class, selector) pair corresponds
- * to a unique GoogleTest result.
+ * Returns a key name for the testInfosByKey dictionary. Each (class, selector)
+ * pair corresponds to a unique GoogleTest result.
  */
 NSString *TestInfoKey(Class testClass, SEL testSelector) {
-  return [NSString
-      stringWithFormat:@"%@.%@", NSStringFromClass(testClass), NSStringFromSelector(testSelector)];
+  return [NSString stringWithFormat:@"%@.%@", NSStringFromClass(testClass),
+                                    NSStringFromSelector(testSelector)];
 }
 
 /**
- * Looks up the testing::TestInfo for this test method and reports on the outcome to XCTest, as if
- * the test actually ran in this method.
+ * Looks up the testing::TestInfo for this test method and reports on the
+ * outcome to XCTest, as if the test actually ran in this method.
  *
- * Note: this function is the implementation for each generated test method. It shouldn't be used
- * directly. The parameter names of self and _cmd match up with the implicit parameters passed to
- * any Objective-C method. Naming them this way here allows XCTAssert and friends to work.
+ * Note: this function is the implementation for each generated test method. It
+ * shouldn't be used directly. The parameter names of self and _cmd match up
+ * with the implicit parameters passed to any Objective-C method. Naming them
+ * this way here allows XCTAssert and friends to work.
  */
 void ReportTestResult(XCTestCase *self, SEL _cmd) {
   NSString *testInfoKey = TestInfoKey([self class], _cmd);
@@ -189,31 +207,38 @@ void ReportTestResult(XCTestCase *self, SEL _cmd) {
     return;
   }
 
-  // Test failed :-(. Record the failure such that XCode will navigate directly to the file:line.
+  // Test failed :-(. Record the failure such that XCode will navigate directly
+  // to the file:line.
   int parts = result->total_part_count();
   for (int i = 0; i < parts; i++) {
     const testing::TestPartResult &part = result->GetTestPartResult(i);
-    [self recordFailureWithDescription:@(part.message())
-                                inFile:@(part.file_name() ? part.file_name() : "")
-                                atLine:(part.line_number() > 0 ? part.line_number() : 0)
-                              expected:YES];
+    [self
+        recordFailureWithDescription:@(part.message())
+                              inFile:@(part.file_name() ? part.file_name() : "")
+                              atLine:(part.line_number() > 0
+                                          ? part.line_number()
+                                          : 0)
+                            expected:YES];
   }
 }
 
 /**
- * Generates a new subclass of XCTestCase for the given GoogleTest TestCase. Each TestInfo (which
- * represents an indivudal test method execution) is translated into a method on the test case.
+ * Generates a new subclass of XCTestCase for the given GoogleTest TestCase.
+ * Each TestInfo (which represents an indivudal test method execution) is
+ * translated into a method on the test case.
  *
  * @param The testing::TestCase of interest to translate.
  * @param A map of TestInfoKeys to testing::TestInfos, populated by this method.
  *
- * @return A new Class that's a subclass of XCTestCase, that's been registered with the Objective-C
- *     runtime.
+ * @return A new Class that's a subclass of XCTestCase, that's been registered
+ * with the Objective-C runtime.
  */
-Class CreateXCTestCaseClass(const testing::TestCase *testCase,
-                            NSMutableDictionary<NSString *, NSValue *> *infoMap) {
+Class CreateXCTestCaseClass(
+    const testing::TestCase *testCase,
+    NSMutableDictionary<NSString *, NSValue *> *infoMap) {
   NSString *testCaseName = ClassNameForTestCase(testCase);
-  Class testClass = objc_allocateClassPair([XCTestCase class], [testCaseName UTF8String], 0);
+  Class testClass =
+      objc_allocateClassPair([XCTestCase class], [testCaseName UTF8String], 0);
 
   // Create a method for each TestInfo.
   int testInfos = testCase->total_test_count();
@@ -223,8 +248,9 @@ Class CreateXCTestCaseClass(const testing::TestCase *testCase,
     NSString *selectorName = SelectorNameForTestInfo(testInfo);
     SEL selector = sel_registerName([selectorName UTF8String]);
 
-    // Use the ReportTestResult function as the method implementation. The v@: indicates it is a
-    // void objective-C method; this must continue to match the signature of ReportTestResult.
+    // Use the ReportTestResult function as the method implementation. The v@:
+    // indicates it is a void objective-C method; this must continue to match
+    // the signature of ReportTestResult.
     IMP method = reinterpret_cast<IMP>(ReportTestResult);
     class_addMethod(testClass, selector, method, "v@:");
 
@@ -238,16 +264,20 @@ Class CreateXCTestCaseClass(const testing::TestCase *testCase,
 }
 
 /**
- * Creates a test suite containing all C++ tests, used when the user starts the GoogleTests class.
+ * Creates a test suite containing all C++ tests, used when the user starts the
+ * GoogleTests class.
  *
- * Note: normally XCTest finds all the XCTestCase classes that are registered with the run time
- * and asks them to create suites for themselves. When a user focuses on the GoogleTests class,
- * XCTest no longer does this so we have to force XCTest to see more tests than it would normally
- * look at so that the indicators in the test navigator update properly.
+ * Note: normally XCTest finds all the XCTestCase classes that are registered
+ * with the run time and asks them to create suites for themselves. When a user
+ * focuses on the GoogleTests class, XCTest no longer does this so we have to
+ * force XCTest to see more tests than it would normally look at so that the
+ * indicators in the test navigator update properly.
  */
 XCTestSuite *CreateAllTestsTestSuite() {
-  XCTestSuite *allTestsSuite = [[XCTestSuite alloc] initWithName:@"All GoogleTest Tests"];
-  [allTestsSuite addTest:[XCTestSuite testSuiteForTestCaseClass:[GoogleTests class]]];
+  XCTestSuite *allTestsSuite =
+      [[XCTestSuite alloc] initWithName:@"All GoogleTest Tests"];
+  [allTestsSuite
+      addTest:[XCTestSuite testSuiteForTestCaseClass:[GoogleTests class]]];
 
   const testing::UnitTest *master = testing::UnitTest::GetInstance();
 
@@ -263,8 +293,8 @@ XCTestSuite *CreateAllTestsTestSuite() {
 }
 
 /**
- * Finds and runs googletest-based tests based on the XCTestConfiguration of the current test
- * invocation.
+ * Finds and runs googletest-based tests based on the XCTestConfiguration of the
+ * current test invocation.
  */
 void RunGoogleTestTests() {
   NSString *masterTestCaseName = NSStringFromClass([GoogleTests class]);
@@ -276,8 +306,9 @@ void RunGoogleTestTests() {
 
   // Convert XCTest's testToRun set to the equivalent --gtest_filter flag.
   //
-  // Note that we only set forceAllTests to YES if the user specifically focused on GoogleTests.
-  // This prevents XCTest double-counting test cases (and failures) when a user asks for all tests.
+  // Note that we only set forceAllTests to YES if the user specifically focused
+  // on GoogleTests. This prevents XCTest double-counting test cases (and
+  // failures) when a user asks for all tests.
   NSSet<NSString *> *allTests = [NSSet setWithObject:masterTestCaseName];
   NSSet<NSString *> *testsToRun = LoadXCTestConfigurationTestsToRun();
   if (testsToRun) {
@@ -307,10 +338,11 @@ void RunGoogleTestTests() {
 
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wunused-result"
-  // RUN_ALL_TESTS by default doesn't want you to ignore its result, but it's safe here. Test
-  // failures are already logged by GoogleTest itself (and then again by XCTest). Test failures are
-  // reported via -recordFailureWithDescription:inFile:atLine:expected: which then causes XCTest
-  // itself to fail the run.
+  // RUN_ALL_TESTS by default doesn't want you to ignore its result, but it's
+  // safe here. Test failures are already logged by GoogleTest itself (and then
+  // again by XCTest). Test failures are reported via
+  // -recordFailureWithDescription:inFile:atLine:expected: which then causes
+  // XCTest itself to fail the run.
   RUN_ALL_TESTS();
 #pragma clang diagnostic pop
 }
@@ -320,7 +352,8 @@ void RunGoogleTestTests() {
 @implementation GoogleTests
 
 + (XCTestSuite *)defaultTestSuite {
-  // Only return all tests beyond GoogleTests if the user is focusing on GoogleTests.
+  // Only return all tests beyond GoogleTests if the user is focusing on
+  // GoogleTests.
   if (forceAllTests) {
     return CreateAllTestsTestSuite();
   } else {
@@ -330,8 +363,8 @@ void RunGoogleTestTests() {
 }
 
 - (void)testGoogleTestsActuallyRun {
-  // This whole mechanism is sufficiently tricky that we should verify that the build actually
-  // plumbed this together correctly.
+  // This whole mechanism is sufficiently tricky that we should verify that the
+  // build actually plumbed this together correctly.
   const testing::UnitTest *master = testing::UnitTest::GetInstance();
   XCTAssertGreaterThan(master->total_test_case_count(), 0);
 }
@@ -339,9 +372,9 @@ void RunGoogleTestTests() {
 @end
 
 /**
- * This class is registered as the NSPrincipalClass in the Firestore_Tests bundle's Info.plist.
- * XCTest instantiates this class to perform one-time setup for the test bundle, as documented
- * here:
+ * This class is registered as the NSPrincipalClass in the Firestore_Tests
+ * bundle's Info.plist. XCTest instantiates this class to perform one-time setup
+ * for the test bundle, as documented here:
  *
  *   https://developer.apple.com/documentation/xctest/xctestobservationcenter
  */