Make ninja work with XCTests for iOS.

Make ninja work with XCTests for iOS.

Update so XCTests are properly built by ninja, after generating
the projects with gyp. This includes correctly handling the bundle
settings from gyp, and then adding the correct paths to access
XCTest.framework.

BUG=
R=justincohen@chromium.org

Committed: https://chromium.googlesource.com/external/gyp/+/863d9a4f4119df3ed6555f4d1c687fb775e92925

[baxley, 2015-10-16 20:01:49]

baxley@chromium.org changed reviewers:
+ justincohen@chromium.org, thakis@chromium.org

[baxley, 2015-10-16 20:01:50]

This is something Justin helped me put together to get XCTests working for iOS.
This is my first gyp CL, so maybe do a quick high-level review for anything
terrible I may have done. Thanks!

-Mike

[Nico, 2015-10-16 20:18:57]

Fancy! Looks good as far as I can tell, but Justin knows iOS much better, so he
should do the real review. From a gyp/ninja pov, I think this looks fine.

https://codereview.chromium.org/1410113002/diff/1/test/ios/xctests/gyptest-xc...
File test/ios/xctests/gyptest-xctests.py (right):

https://codereview.chromium.org/1410113002/diff/1/test/ios/xctests/gyptest-xc...
test/ios/xctests/gyptest-xctests.py:12: 
you probably want an

if sys.platform == 'darwin':

here and put everything below into that block, else the test will try to run on
win and linux too

https://codereview.chromium.org/1410113002/diff/1/test/ios/xctests/gyptest-xc...
test/ios/xctests/gyptest-xctests.py:13: #test =
TestGyp.TestGyp(formats=['ninja'])
remove

https://codereview.chromium.org/1410113002/diff/1/test/ios/xctests/gyptest-xc...
test/ios/xctests/gyptest-xctests.py:15: #test.formats = ['ninja', 'xcode-ninja']
remove

[baxley, 2015-10-16 20:59:20]

Thanks for the quick review!

Justin, let me know what you think.

https://codereview.chromium.org/1410113002/diff/1/test/ios/xctests/gyptest-xc...
File test/ios/xctests/gyptest-xctests.py (right):

https://codereview.chromium.org/1410113002/diff/1/test/ios/xctests/gyptest-xc...
test/ios/xctests/gyptest-xctests.py:12: 
On 2015/10/16 20:18:57, Nico wrote:
> you probably want an
> 
> if sys.platform == 'darwin':
> 
> here and put everything below into that block, else the test will try to run
on
> win and linux too

Done.

https://codereview.chromium.org/1410113002/diff/1/test/ios/xctests/gyptest-xc...
test/ios/xctests/gyptest-xctests.py:13: #test =
TestGyp.TestGyp(formats=['ninja'])
On 2015/10/16 20:18:57, Nico wrote:
> remove

Done.

https://codereview.chromium.org/1410113002/diff/1/test/ios/xctests/gyptest-xc...
test/ios/xctests/gyptest-xctests.py:15: #test.formats = ['ninja', 'xcode-ninja']
On 2015/10/16 20:18:57, Nico wrote:
> remove

Done.

[justincohen, 2015-10-16 21:15:53]

LGTM!  I can't wait to rewrite all our tests into xctests =P

[baxley, 2015-10-16 22:27:41]

On 2015/10/16 21:15:53, justincohen wrote:
> LGTM!  I can't wait to rewrite all our tests into xctests =P

Nico,

Did you have any other questions, or should I just go with Justin's review?

[baxley, 2015-10-17 00:10:49]

On 2015/10/16 22:27:41, baxley wrote:
> On 2015/10/16 21:15:53, justincohen wrote:
> > LGTM!  I can't wait to rewrite all our tests into xctests =P
> 
> Nico,
> 
> Did you have any other questions, or should I just go with Justin's review?

I got an error...
error: failed to push some refs to
'https://chromium.googlesource.com/external/gyp'

Am I likely doing something wrong? Or perhaps I don't have commit access to this
repo?

Thanks for all the help!
-Mike

[Nico, 2015-10-17 03:06:04]

I was about to land this and ran the new test locally. It doesn't pass for me:

gyp thakis$ python gyptest.py test/ios/xctests/gyptest-xctests.py 
PYTHONPATH=/Users/thakis/src/chrome/src/tools/gyp/test/lib
TESTGYP_FORMAT=make
/usr/bin/python test/ios/xctests/gyptest-xctests.py
Invalid test for 'make' format; skipping test.
NO RESULT
TESTGYP_FORMAT=ninja
/usr/bin/python test/ios/xctests/gyptest-xctests.py
PASSED
TESTGYP_FORMAT=xcode
/usr/bin/python test/ios/xctests/gyptest-xctests.py
None returned 65
STDOUT =========================================================================
Build settings from command line:
    SYMROOT = $SRCROOT/build

=== BUILD TARGET app_under_test OF PROJECT xctests WITH THE DEFAULT
CONFIGURATION (Default) ===

Check dependencies
Code Sign error: No code signing identities found: No valid signing identities
(i.e. certificate and private key pair) were found.


STDERR =========================================================================
** BUILD FAILED **


The following build commands failed:
	Check dependencies
(1 failure)

FAILED test at line 423 of
/Users/thakis/src/chrome/src/tools/gyp/test/lib/TestCommon.py (_complete)
	from line 532 of /Users/thakis/src/chrome/src/tools/gyp/test/lib/TestCommon.py
(run)
	from line 307 of /Users/thakis/src/chrome/src/tools/gyp/test/lib/TestGyp.py
(run)
	from line 1012 of /Users/thakis/src/chrome/src/tools/gyp/test/lib/TestGyp.py
(build)
	from line 21 of test/ios/xctests/gyptest-xctests.py
TESTGYP_FORMAT=xcode-ninja
/usr/bin/python test/ios/xctests/gyptest-xctests.py
PASSED

Failed the following test:
	test/ios/xctests/gyptest-xctests.py

No result from the following test:
	test/ios/xctests/gyptest-xctests.py

[justincohen, 2015-10-17 03:12:39]

Ah, baxely@ take a look at gyptest-per-config-settings.py and look at the
HasCerts(): section.  The bots, and thakis@ it seems, do not have certs
installed and cannot do device builds.

[baxley, 2015-10-27 18:38:23]

On 2015/10/17 03:12:39, justincohen wrote:
> Ah, baxely@ take a look at gyptest-per-config-settings.py and look at the
> HasCerts(): section.  The bots, and thakis@ it seems, do not have certs
> installed and cannot do device builds.

I made a few changes to get it working for xcode on my system. Justin helped me
a bit last week, and some of the tests were failing for xcode prior to this CL.
I verified that I get the same failures in test/ios with and without my CL, and
all my tests in test/ios/xctest pass (except make which is no result).

Let me know what you think.

-Mike

[justincohen, 2015-10-27 19:38:49]

https://codereview.chromium.org/1410113002/diff/60001/test/ios/xctests/gyptes...
File test/ios/xctests/gyptest-xctests.py (right):

https://codereview.chromium.org/1410113002/diff/60001/test/ios/xctests/gyptes...
test/ios/xctests/gyptest-xctests.py:29: if test.format == 'xcode' and
HasCerts():
Don't you need to remove Default-iphoneos if there are no certs?

[baxley, 2015-11-04 22:10:41]

Sorry for the delay, I have a couple of questions.

I'm open to making changes to xctest.gyp if that would make it better.

https://codereview.chromium.org/1410113002/diff/60001/test/ios/xctests/gyptes...
File test/ios/xctests/gyptest-xctests.py (right):

https://codereview.chromium.org/1410113002/diff/60001/test/ios/xctests/gyptes...
test/ios/xctests/gyptest-xctests.py:29: if test.format == 'xcode' and
HasCerts():
On 2015/10/27 19:38:49, justincohen wrote:
> Don't you need to remove Default-iphoneos if there are no certs?

xctest.gyp sets SDKROOT to iphoneos when GENERATOR is xcode. Would this cover
that?

Would it be sufficient to just use iphonesimulator for both?

[baxley, 2015-11-18 16:46:17]

Justin,

I made some changes to just have it do Debug-iphonesimulator, then if it has
certs and is xcode to add a config for Debug-iphonesimulator. However something
is going wrong with this build and other tests that do xcodebuild for device, so
I filed a bug and disabled that part.

I ran this patch and it passed on a machine with and without certs.

[baxley, 2015-11-18 16:46:57]

https://codereview.chromium.org/1410113002/diff/100001/pylib/gyp/xcode_emulat...
File pylib/gyp/xcode_emulation.py (right):

https://codereview.chromium.org/1410113002/diff/100001/pylib/gyp/xcode_emulat...
pylib/gyp/xcode_emulation.py:3: # found in the LICENSE file.
I also got a presubmit warning about not having a Chromium copyright, does that
need to be updated?

[justincohen, 2015-11-19 18:37:36]

https://codereview.chromium.org/1410113002/diff/100001/test/ios/xctests/gypte...
File test/ios/xctests/gyptest-xctests.py (right):

https://codereview.chromium.org/1410113002/diff/100001/test/ios/xctests/gypte...
test/ios/xctests/gyptest-xctests.py:33: p =
test.built_file_path('app_under_test.app/app_under_test')
What is this for?

https://codereview.chromium.org/1410113002/diff/100001/test/ios/xctests/gypte...
test/ios/xctests/gyptest-xctests.py:38:
test.built_file_must_exist('../../xctests.xcodeproj')
Not sure what the value of checking this is for an xctests test?

[baxley, 2015-11-19 21:20:54]

https://codereview.chromium.org/1410113002/diff/100001/test/ios/xctests/gypte...
File test/ios/xctests/gyptest-xctests.py (right):

https://codereview.chromium.org/1410113002/diff/100001/test/ios/xctests/gypte...
test/ios/xctests/gyptest-xctests.py:33: p =
test.built_file_path('app_under_test.app/app_under_test')
On 2015/11/19 18:37:36, justincohen wrote:
> What is this for?

Shouldn't be there, removed.

https://codereview.chromium.org/1410113002/diff/100001/test/ios/xctests/gypte...
test/ios/xctests/gyptest-xctests.py:38:
test.built_file_must_exist('../../xctests.xcodeproj')
On 2015/11/19 18:37:36, justincohen wrote:
> Not sure what the value of checking this is for an xctests test?

Probably not needed, removed.

https://codereview.chromium.org/1410113002/diff/100001/test/ios/xctests/gypte...
test/ios/xctests/gyptest-xctests.py:41:
test.built_file_must_exist('obj/AppTests/app_tests.AppTests.x86_64.o')
Is there value in checking for the .o files to make sure things are compiled?
It's doable for the other formats, but those files are in much deeper
directories that may be dependent upon the configuration (Debug,
Debug-iphonesimulator, ...).

[justincohen, 2015-11-20 16:14:51]

https://codereview.chromium.org/1410113002/diff/100001/test/ios/xctests/gypte...
File test/ios/xctests/gyptest-xctests.py (right):

https://codereview.chromium.org/1410113002/diff/100001/test/ios/xctests/gypte...
test/ios/xctests/gyptest-xctests.py:41:
test.built_file_must_exist('obj/AppTests/app_tests.AppTests.x86_64.o')
On 2015/11/19 21:20:54, baxley wrote:
> Is there value in checking for the .o files to make sure things are compiled?
> It's doable for the other formats, but those files are in much deeper
> directories that may be dependent upon the configuration (Debug,
> Debug-iphonesimulator, ...).

I think there is.  There's always the chance something in Xcode changes and the
build does something unexpected.  Is it a problem to add this for xcode?

[baxley, 2015-11-20 21:33:47]

Should I be concerned about presubmit warnings about having a Chromium
copyright, instead of Google?

https://codereview.chromium.org/1410113002/diff/100001/test/ios/xctests/gypte...
File test/ios/xctests/gyptest-xctests.py (right):

https://codereview.chromium.org/1410113002/diff/100001/test/ios/xctests/gypte...
test/ios/xctests/gyptest-xctests.py:41:
test.built_file_must_exist('obj/AppTests/app_tests.AppTests.x86_64.o')
On 2015/11/20 16:14:51, justincohen wrote:
> On 2015/11/19 21:20:54, baxley wrote:
> > Is there value in checking for the .o files to make sure things are
compiled?
> > It's doable for the other formats, but those files are in much deeper
> > directories that may be dependent upon the configuration (Debug,
> > Debug-iphonesimulator, ...).
> 
> I think there is.  There's always the chance something in Xcode changes and
the
> build does something unexpected.  Is it a problem to add this for xcode?

Okay, I added some code to look for the xcodebuild output. Let me know if you
have a cleaner way to find the fine.

[justincohen, 2015-11-20 21:40:26]

Not sure about the presubmit question, Nico do you know?

The rest LGTM

[baxley, 2015-12-01 19:12:17]

On 2015/11/20 21:40:26, justincohen wrote:
> Not sure about the presubmit question, Nico do you know?
> 
> The rest LGTM

Hey Nico,

I just had one outstanding question about the copyright. Presubmit is asking for
Chromium, but a lot of the files have Google (even prior to my change). Does
this mean something might be wrong with how presubmit is configured for me?
Should I update the copyright?

Thanks!
-Mike

[Nico, 2015-12-01 19:42:45]

use Chromium for new files

[baxley, 2015-12-03 17:21:27]

On 2015/12/01 19:42:45, Nico wrote:
> use Chromium for new files

I changed all the new files to Chromium. Thanks for all the help.

[baxley, 2015-12-07 22:43:11]

On 2015/12/03 17:21:27, baxley wrote:
> On 2015/12/01 19:42:45, Nico wrote:
> > use Chromium for new files
> 
> I changed all the new files to Chromium. Thanks for all the help.

Justin,

Did you get a chance to try and land this? Did you find any issues?

[justincohen, 2015-12-09 00:04:56]

Description was changed from

==========
Make ninja work with XCTests for iOS.

Update so XCTests are properly built by ninja, after generating
the projects with gyp. This includes correctly handling the bundle
settings from gyp, and then adding the correct paths to access
XCTest.framework.

BUG=
==========

to

==========
Make ninja work with XCTests for iOS.

Update so XCTests are properly built by ninja, after generating
the projects with gyp. This includes correctly handling the bundle
settings from gyp, and then adding the correct paths to access
XCTest.framework.

BUG=
R=justincohen@chromium.org

Committed:
https://chromium.googlesource.com/external/gyp/+/863d9a4f4119df3ed6555f4d1c68...
==========

[justincohen, 2015-12-09 00:04:57]

Committed patchset #10 (id:180001) manually as
863d9a4f4119df3ed6555f4d1c687fb775e92925 (presubmit successful).

[Nico, 2015-12-14 19:56:41]

I noticed that this landed but DEPS in chromium wasn't updated after that.
Please always roll gyp after landing changes to it, to make sure the change
won't break chromium. (I'll include it in the gyp roll I'm doing now.)

[baxley, 2015-12-14 20:34:19]

On 2015/12/14 19:56:41, Nico wrote:
> I noticed that this landed but DEPS in chromium wasn't updated after that.
> Please always roll gyp after landing changes to it, to make sure the change
> won't break chromium. (I'll include it in the gyp roll I'm doing now.)

I sent one out for review last week after Justin landed it
https://codereview.chromium.org/1512203005/