Add functionality to find xctest files under application target.

tools/gn/xcode_writer.cc

Index: tools/gn/xcode_writer.cc
diff --git a/tools/gn/xcode_writer.cc b/tools/gn/xcode_writer.cc
index 6e5042865aa1dc746382e68c191f76d33edb9dd1..0e3dfda5f11c16a76d6637323d879e077f0fd5ae 100644
--- a/tools/gn/xcode_writer.cc
+++ b/tools/gn/xcode_writer.cc
@@ -31,6 +31,11 @@
 
 namespace {
 
+const char kXCTestTargetNamePostfix[] = "_module";
+const char kEarlGreyFileNameIdentifier[] = "egtest.mm";
+
+typedef std::unordered_map<const Target*, Target::FileList> TargetToFileList;

[sdefresne, 2016/12/14 13:00:03]

nit: it is recommend to use "using" instead of "typedef"

using TargetToFileList = std::unordered_map<const Target*, Target::FileList>;

[liaoyuke, 2016/12/15 00:03:11]

Done.

+
 struct SafeEnvironmentVariableInfo {
   const char* name;
   bool capture_at_generation;
@@ -87,6 +92,92 @@ std::string GetBuildScript(const std::string& target_name,
   return script.str();
 }
 
+// Append |file_list_extra| to |file_list_base| and remove duplicate elements.
+void AppendAndRemoveDuplicates(Target::FileList* file_list_base,
+                               const Target::FileList& file_list_extra) {
+  file_list_base->insert(file_list_base->end(), file_list_extra.begin(),
+                         file_list_extra.end());
+  std::sort(file_list_base->begin(), file_list_base->end());
+  file_list_base->erase(
+      std::unique(file_list_base->begin(), file_list_base->end()),
+      file_list_base->end());
+}
+
+// Find the list of earl grey files recursively under |target|.
+void GetEarlGreyFiles(const Target* target,
+                      TargetToFileList* eg_files_per_target) {
+  // Early return if already visited and processed.
+  if (eg_files_per_target->find(target) != eg_files_per_target->end()) {

[sdefresne, 2016/12/14 13:00:03]

No braces here (since there are not used in the rest of the file).

[liaoyuke, 2016/12/15 00:03:11]

Done.

+    return;
+  }
+
+  Target::FileList eg_files;
+  for (const SourceFile& file : target->sources()) {
+    if (base::EndsWith(file.GetName(), kEarlGreyFileNameIdentifier,
+                       base::CompareCase::SENSITIVE)) {
+      eg_files.push_back(file);
+    }
+  }
+
+  // Call recursively on public and private deps.
+  for (const auto& target : target->public_deps()) {
+    GetEarlGreyFiles(target.ptr, eg_files_per_target);
+    Target::FileList deps_eg_files = eg_files_per_target->at(target.ptr);
+    if (!deps_eg_files.empty()) {

[sdefresne, 2016/12/14 13:00:02]

No braces here.

[liaoyuke, 2016/12/15 00:03:11]

Done.

+      AppendAndRemoveDuplicates(&eg_files, deps_eg_files);

[sdefresne, 2016/12/14 13:00:03]

I think it would be better to just add everything and remove duplicates at the
end.

  for (...) {
    ...
    const Target::FileList& deps_eg_files = (*eg_files_per_target)[target.ptr];
    eg_files.insert(eg_files.end(), deps_eg_files.begin(), deps_eg_files.end());
  }

Since you are already removing the duplicates at the end, there is no point
removing them at each iteration of the loop.

[liaoyuke, 2016/12/15 00:03:11]

Done.

+    }
+  }
+
+  for (const auto& target : target->private_deps()) {
+    GetEarlGreyFiles(target.ptr, eg_files_per_target);
+    Target::FileList deps_eg_files = eg_files_per_target->at(target.ptr);
+    if (!deps_eg_files.empty()) {
+      AppendAndRemoveDuplicates(&eg_files, deps_eg_files);
+    }
+  }
+
+  // Sort eg_files to remove duplicates.
+  std::sort(eg_files.begin(), eg_files.end());
+  eg_files.erase(std::unique(eg_files.begin(), eg_files.end()), eg_files.end());
+
+  eg_files_per_target->insert(std::make_pair(target, eg_files));
+}
+
+// Find the list of earl grey files recursively under each of the application

[sdefresne, 2016/12/14 13:00:02]

nit: Find -> Finds

[liaoyuke, 2016/12/15 00:03:11]

Done.

+// and xctest bundle pair.
+void GetEarlGreyFilesForAll(

[sdefresne, 2016/12/14 13:00:02]

I'm not sure we need to check source file in xctest_targets since in Chromium we
only put the tests in the application itself, not in the module. Can you check
whether you get the same result if you do not iterate on xctest_targets, and if
this is the case, just remove the xctest_targets from this method?

[liaoyuke, 2016/12/15 00:03:11]

You are right, I have double-checked, there is no earl grey files under xctest
targets.

+    const std::vector<const Target*>& xctest_targets,
+    const std::vector<const Target*>& application_targets,
+    std::vector<Target::FileList>* file_lists) {

[sdefresne, 2016/12/14 13:00:03]

DCHECK_EQ(xctest_targets.size(), application_targets.size());

[liaoyuke, 2016/12/15 00:03:11]

Acknowledged.

+  TargetToFileList eg_files_per_target;
+
+  for (size_t i = 0; i < xctest_targets.size(); ++i) {
+    GetEarlGreyFiles(xctest_targets[i], &eg_files_per_target);
+    GetEarlGreyFiles(application_targets[i], &eg_files_per_target);
+
+    Target::FileList eg_files;
+    AppendAndRemoveDuplicates(&eg_files,
+                              eg_files_per_target.at(xctest_targets[i]));
+    AppendAndRemoveDuplicates(&eg_files,
+                              eg_files_per_target.at(application_targets[i]));
+    file_lists->push_back(eg_files);
+  }
+}
+
+// Returns the corresponding application target given a XCTest target. Returns

[sdefresne, 2016/12/14 13:00:03]

I would change this function to DCHECK if it does not find the application
target for an XCTest.

// Returns the application target given an XCTest target.
const Target* FindApplicationTarget(const Target* xctest_target,
                                    const std::vector<const Target*>& targets) {
  DCHECK(IsXCTestTarget(xctest_target));
  DCHECK(base::EndsWith(xctest_target->label().name(),
kXCTestTargetNamePostfix));
  std::string application_target_name = xctest_target->label().name().substr(
      0, xctest_target->label().name().size() -
strlen(kXCTestTargetNamePostfix));
  for (const Target* target : targets) {
    if (target->label().name() == application_target_name) {
      return target;
    }
  }
  NOTREACHED();
  return nullptr;
}

[liaoyuke, 2016/12/15 00:03:11]

Thank you for the note. I will this logic when I write the function to find
xctest target given application target.

[liaoyuke, 2016/12/15 00:03:11]

Acknowledged.

+// NULL if not found.
+const Target* FindApplicationTarget(const Target* xctest_target,
+                                    const std::vector<const Target*>& targets) {
+  for (const Target* target : targets) {
+    if ((target->label().name() + kXCTestTargetNamePostfix)
+            .compare(xctest_target->label().name()) == 0) {
+      return target;
+    }
+  }
+
+  return NULL;
+}
+
 class CollectPBXObjectsPerClassHelper : public PBXObjectVisitor {
  public:
   CollectPBXObjectsPerClassHelper() {}
@@ -267,6 +358,27 @@ bool XcodeWriter::FilterTargets(const BuildSettings* build_settings,
   return true;
 }
 
+// static
+void XcodeWriter::FilterXCTestTargets(
+    const std::vector<const Target*>& targets,
+    std::vector<const Target*>* xctest_targets,
+    std::vector<const Target*>* application_targets) {
+  // Filter out all targets of type CREATE_BUNDLE and whose bundle_data has
+  // product_type: "com.apple.product-type.bundle.unit-test".
+  for (const Target* target : targets) {
+    if (target->output_type() != Target::CREATE_BUNDLE) {

[sdefresne, 2016/12/14 13:00:02]

nit: I would use an helper method IsXCTestTarget here:

namespace {
bool IsXCTestTarget(const Target* target) {
  return target->output_type() == Target::CREATE_BUNDLE &&
      target->bundle_data().product_type() ==
"com.apple.product-type.bundle.unit-test";
}
}

  ...
  for (const Target* target : targets) {
    if (!IsXCTestTarget(target))
      continue;

    xctest_targets->push_back(target);
    application_targets->ush_back(FindApplicationTarget(target, targets));
  }

+      continue;
+    }
+    if (target->bundle_data().product_type().compare(
+            "com.apple.product-type.bundle.unit-test") == 0) {
+      xctest_targets->push_back(target);
+      const Target* application_target = FindApplicationTarget(target, targets);
+      DCHECK(application_target);
+      application_targets->push_back(application_target);
+    }
+  }
+}
+
 void XcodeWriter::CreateProductsProject(
     const std::vector<const Target*>& targets,
     const PBXAttributes& attributes,
@@ -279,6 +391,16 @@ void XcodeWriter::CreateProductsProject(
   std::unique_ptr<PBXProject> main_project(
       new PBXProject("products", config_name, source_path, attributes));
 
+  // Filter XCTest targets and their corresponding application targets and find
+  // list of earl grey test files recursively under the pairs.
+  std::vector<const Target*> xctest_targets;
+  std::vector<const Target*> application_targets;
+  std::vector<Target::FileList> earl_grey_file_lists;
+  XcodeWriter::FilterXCTestTargets(targets, &xctest_targets,
+                                   &application_targets);
+  GetEarlGreyFilesForAll(xctest_targets, application_targets,
+                         &earl_grey_file_lists);
+
   std::string build_path;
   std::unique_ptr<base::Environment> env(base::Environment::Create());