Associate xctest files with corresponding xctest targets.

Associate xctest files with corresponding xctest targets.

Previously, our generated Xcode project has the issue that Xcode couldn't find
our xctests. And to fix it, one must find the list of xctest files under each of
the application targets and then associate them with the corresponding xctest
target.

This CL associates xctest files with xctest targets and because our compilation
is done via ninja, it also adds '-help' as per file compiler flags to each
xctest file reference to prevent Xcode from trying to compile the xctest files.

BUG=614818
Review-Url: https://codereview.chromium.org/2616773003
Cr-Commit-Position: refs/heads/master@{#442120}
Committed: https://chromium.googlesource.com/chromium/src/+/4ce240b3ecb8ff7d7b902f2ccdf361411915610a

[liaoyuke, 2017-01-05 06:17:24]

Description was changed from

==========
Associate xctest files with corresponding xctest targets.

Previously, our generated Xcode project has the issue that Xcode couldn't find
our xctests. And to fix it, one must find the list of xctest files under each of
the application targets and then associate them with the corresponding xctest
target.

This CL associates xctest files with xctest targets and because our compilation
is done via ninja, it also adds '-help' as per file compiler flags to each
xctest file reference to prevent Xcode from trying to compile the xctest files.

BUG=614818
==========

to

==========
Associate xctest files with corresponding xctest targets.

Previously, our generated Xcode project has the issue that Xcode couldn't find
our xctests. And to fix it, one must find the list of xctest files under each of
the application targets and then associate them with the corresponding xctest
target.

This CL associates xctest files with xctest targets and because our compilation
is done via ninja, it also adds '-help' as per file compiler flags to each
xctest file reference to prevent Xcode from trying to compile the xctest files.

BUG=614818
==========

[liaoyuke, 2017-01-05 06:17:24]

liaoyuke@chromium.org changed reviewers:
+ justincohen@chromium.org, sdefresne@chromium.org

[liaoyuke, 2017-01-05 06:18:06]

PTAL!

One last CL :)

Thank you very much!

[sdefresne, 2017-01-05 12:14:02]

https://codereview.chromium.org/2616773003/diff/1/tools/gn/xcode_writer.cc
File tools/gn/xcode_writer.cc (right):

https://codereview.chromium.org/2616773003/diff/1/tools/gn/xcode_writer.cc#ne...
tools/gn/xcode_writer.cc:215: if
(xctest_file_to_application_targets->find(source) ==
I think you can avoid doing the search twice by using the fact that insert
returns a std::pair<iterator, bool>:

  auto iter = xctest_file_to_application_targets->find(source);
  if (iter == xctest_file_to_application_targets->end()) {
    iter = xctest_file_to_application_targets->insert(
        std::make_pair(source, std::vector<const Target*>())).first;
  }
  iter->push_back(xctest_application_target);

https://codereview.chromium.org/2616773003/diff/1/tools/gn/xcode_writer.cc#ne...
tools/gn/xcode_writer.cc:496: // populate
|xctest_application_to_module_native_target| for XCTest
s/populate/Populate/

https://codereview.chromium.org/2616773003/diff/1/tools/gn/xcode_writer.cc#ne...
tools/gn/xcode_writer.cc:511: FileToTargets xctest_file_to_application_targets(
Please move this to be a real free function in the anonymous namespace instead
of a lambda.

[liaoyuke, 2017-01-05 17:43:12]

Addressed comments. Please take another look.

Thank you very much!

https://codereview.chromium.org/2616773003/diff/1/tools/gn/xcode_writer.cc
File tools/gn/xcode_writer.cc (right):

https://codereview.chromium.org/2616773003/diff/1/tools/gn/xcode_writer.cc#ne...
tools/gn/xcode_writer.cc:215: if
(xctest_file_to_application_targets->find(source) ==
On 2017/01/05 12:14:02, sdefresne wrote:
> I think you can avoid doing the search twice by using the fact that insert
> returns a std::pair<iterator, bool>:
> 
>   auto iter = xctest_file_to_application_targets->find(source);
>   if (iter == xctest_file_to_application_targets->end()) {
>     iter = xctest_file_to_application_targets->insert(
>         std::make_pair(source, std::vector<const Target*>())).first;
>   }
>   iter->push_back(xctest_application_target);

Done.

https://codereview.chromium.org/2616773003/diff/1/tools/gn/xcode_writer.cc#ne...
tools/gn/xcode_writer.cc:496: // populate
|xctest_application_to_module_native_target| for XCTest
On 2017/01/05 12:14:02, sdefresne wrote:
> s/populate/Populate/

Done.

https://codereview.chromium.org/2616773003/diff/1/tools/gn/xcode_writer.cc#ne...
tools/gn/xcode_writer.cc:511: FileToTargets xctest_file_to_application_targets(
On 2017/01/05 12:14:02, sdefresne wrote:
> Please move this to be a real free function in the anonymous namespace instead
> of a lambda.

Done.

[sdefresne, 2017-01-06 10:45:26]

lgtm

[liaoyuke, 2017-01-06 16:21:48]

liaoyuke@chromium.org changed reviewers:
+ dpranke@chromium.org

[liaoyuke, 2017-01-06 16:22:25]

On 2017/01/06 10:45:26, sdefresne wrote:
> lgtm

Thank you very much for reviewing!

Hello Dirk,

Please take a look for owner's approval.

Thanks,
Yuke

[Dirk Pranke, 2017-01-07 00:26:31]

lgtm.

[liaoyuke, 2017-01-07 00:27:49]

The CQ bit was checked by liaoyuke@chromium.org

[commit-bot: I haz the power, 2017-01-07 00:28:11]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-07 01:00:03]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1483748869506840,
"parent_rev": "c725a146113b550b42700e2cf9bccdd522c25f91", "commit_rev":
"4ce240b3ecb8ff7d7b902f2ccdf361411915610a"}

[commit-bot: I haz the power, 2017-01-07 01:00:31]

Description was changed from

==========
Associate xctest files with corresponding xctest targets.

Previously, our generated Xcode project has the issue that Xcode couldn't find
our xctests. And to fix it, one must find the list of xctest files under each of
the application targets and then associate them with the corresponding xctest
target.

This CL associates xctest files with xctest targets and because our compilation
is done via ninja, it also adds '-help' as per file compiler flags to each
xctest file reference to prevent Xcode from trying to compile the xctest files.

BUG=614818
==========

to

==========
Associate xctest files with corresponding xctest targets.

Previously, our generated Xcode project has the issue that Xcode couldn't find
our xctests. And to fix it, one must find the list of xctest files under each of
the application targets and then associate them with the corresponding xctest
target.

This CL associates xctest files with xctest targets and because our compilation
is done via ninja, it also adds '-help' as per file compiler flags to each
xctest file reference to prevent Xcode from trying to compile the xctest files.

BUG=614818

Review-Url: https://codereview.chromium.org/2616773003
Cr-Commit-Position: refs/heads/master@{#442120}
Committed:
https://chromium.googlesource.com/chromium/src/+/4ce240b3ecb8ff7d7b902f2ccdf3...
==========

[commit-bot: I haz the power, 2017-01-07 01:00:35]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/4ce240b3ecb8ff7d7b902f2ccdf3...