Add TestGypXcodeNinja to run tests against the xcode-ninja generator

Add TestGypXcodeNinja to run tests against the xcode-ninja generator

Use:

    ./gyptest.py -f xcode-ninja <tests>

BUG=gyp:443
TEST=Run
    ./gyptest.py -f xcode-ninja -a

Expected: No test failures.

Patch by Tobias Hahn <tobias.hahn@ableton.com>;.

R=mark@chromium.org, sdefresne@chromium.org, justincohen@chromium.org

Committed: https://code.google.com/p/gyp/source/detail?r=2017

[sdefresne, 2014-07-30 12:17:54]

+mark

I think this LGTM but I don't know the test framework of gyp to say if it is
enough or not, so please wait for Mark LGTM.

[Mark Mentovai, 2014-07-30 13:21:05]

LGTM

[sdefresne, 2014-08-01 13:03:16]

After pushing the two previous patches fixing the unit tests, I tried to run the
unit tests before submitting this one.

First, running "python gyptest.py -a" does not run the tests with for the
"xcode-ninja" generator. This should probably be fixed.

Second, running "python gyptest.py -a -f xcode-ninja" to force the use of the
"xcode-ninja" generator results in 89 tests failing (for reference there is only
one test failing when using the other generators). It's possible that some of
those tests don't make sense when using xcode-ninja, but could you take a look
and see if you can disable those test for xcode-ninja or fix them?

[Tobias, 2014-08-01 15:58:07]

On 2014/08/01 13:03:16, sdefresne wrote:
> After pushing the two previous patches fixing the unit tests, I tried to run
the
> unit tests before submitting this one.
> 
> First, running "python gyptest.py -a" does not run the tests with for the
> "xcode-ninja" generator. This should probably be fixed.
> 
> Second, running "python gyptest.py -a -f xcode-ninja" to force the use of the
> "xcode-ninja" generator results in 89 tests failing (for reference there is
only
> one test failing when using the other generators). It's possible that some of
> those tests don't make sense when using xcode-ninja, but could you take a look
> and see if you can disable those test for xcode-ninja or fix them?

Thanks for the heads up. I only ran "gyptest -a" and "gyptest -f xcode-ninja
tests/generator-output" locally. I'll have a look and try to fix this asap.

[Tobias, 2014-08-28 14:56:33]

PTAL

[sdefresne, 2014-09-02 15:13:06]

Please update the description too.

https://codereview.chromium.org/421453003/diff/60001/test/build-option/gyptes...
File test/build-option/gyptest-build.py (right):

https://codereview.chromium.org/421453003/diff/60001/test/build-option/gyptes...
test/build-option/gyptest-build.py:23: # The xcode-ninja generator doesn't
support --build
Is this a bug or by design? If it is a bug, I think an issue should be created
in the issue tracker, and a link to it should be added there, so that it get
fixed.

https://codereview.chromium.org/421453003/diff/60001/test/compiler-override/g...
File test/compiler-override/gyptest-compiler-env.py (right):

https://codereview.chromium.org/421453003/diff/60001/test/compiler-override/g...
test/compiler-override/gyptest-compiler-env.py:113: # The xcode generator chokes
on the 'host' toolset.
Is this a bug or by design?

https://codereview.chromium.org/421453003/diff/60001/test/compiler-override/g...
File test/compiler-override/gyptest-compiler-global-settings.py (right):

https://codereview.chromium.org/421453003/diff/60001/test/compiler-override/g...
test/compiler-override/gyptest-compiler-global-settings.py:46: # The xcode
generator chokes on the 'host' toolset.
Same question here.

https://codereview.chromium.org/421453003/diff/60001/test/generator-output/gy...
File test/generator-output/gyptest-actions.py (right):

https://codereview.chromium.org/421453003/diff/60001/test/generator-output/gy...
test/generator-output/gyptest-actions.py:16: # Bug: xcode-ninja doesn't respect
--generator-output
Please file a bug (if it does not exists) and add reference here, something
like:

# TODO(tobias.hahn): xcode-ninja doesn't respect --generator-output,
http://crbug.com/xxxxxx

Or if this is by design, update instead the formats parameter of the
TestGyp.TestGyp() call above to exclude xcode-ninja too.

https://codereview.chromium.org/421453003/diff/60001/test/generator-output/gy...
File test/generator-output/gyptest-copies.py (right):

https://codereview.chromium.org/421453003/diff/60001/test/generator-output/gy...
test/generator-output/gyptest-copies.py:17: # Bug: xcode-ninja doesn't respect
--generator-output
Same here.

https://codereview.chromium.org/421453003/diff/60001/test/generator-output/gy...
File test/generator-output/gyptest-mac-bundle.py (right):

https://codereview.chromium.org/421453003/diff/60001/test/generator-output/gy...
test/generator-output/gyptest-mac-bundle.py:18: # Bug: xcode-ninja doesn't
respect --generator-output
Same here.

https://codereview.chromium.org/421453003/diff/60001/test/generator-output/gy...
File test/generator-output/gyptest-relocate.py (right):

https://codereview.chromium.org/421453003/diff/60001/test/generator-output/gy...
test/generator-output/gyptest-relocate.py:17: # Bug: xcode-ninja doesn't respect
--generator-output
Same here.

https://codereview.chromium.org/421453003/diff/60001/test/generator-output/gy...
File test/generator-output/gyptest-rules.py (right):

https://codereview.chromium.org/421453003/diff/60001/test/generator-output/gy...
test/generator-output/gyptest-rules.py:16: # Bug: xcode-ninja doesn't respect
--generator-output
Same here.

https://codereview.chromium.org/421453003/diff/60001/test/generator-output/gy...
File test/generator-output/gyptest-subdir2-deep.py (right):

https://codereview.chromium.org/421453003/diff/60001/test/generator-output/gy...
test/generator-output/gyptest-subdir2-deep.py:18: # Bug: xcode-ninja doesn't
respect --generator-output
Same here.

https://codereview.chromium.org/421453003/diff/60001/test/generator-output/gy...
File test/generator-output/gyptest-symlink.py (right):

https://codereview.chromium.org/421453003/diff/60001/test/generator-output/gy...
test/generator-output/gyptest-symlink.py:21: # Bug: xcode-ninja doesn't respect
--generator-output
Same here.

https://codereview.chromium.org/421453003/diff/60001/test/generator-output/gy...
File test/generator-output/gyptest-top-all.py (right):

https://codereview.chromium.org/421453003/diff/60001/test/generator-output/gy...
test/generator-output/gyptest-top-all.py:18: # Bug: xcode-ninja doesn't respect
--generator-output
Same here.

https://codereview.chromium.org/421453003/diff/60001/test/lib/TestGyp.py
File test/lib/TestGyp.py (right):

https://codereview.chromium.org/421453003/diff/60001/test/lib/TestGyp.py#newc...
test/lib/TestGyp.py:291: run_args.extend(self.extra_args)
I don't really like the fact that you add to change each and every test case to
add "-G xcode_ninja_target_pattern=^${something}$" as additional arguments, with
a default value of .* for something.

What about adding the following here:

  # Default xcode_ninja_target_pattern to ^.*$ to fix xcode-ninja tests
  xcode_ninja_target_pattern = kw.pop('xcode_ninja_target_pattern', '^.*$')
  run_args.extend(['-G', 'xcode_ninja_target_pattern=%s' %
xcode_ninja_target_pattern)

Then you can omit the '-G', 'xcode_ninja_target_pattern=^whatever$' in most of
the call of run_gyp, and pass it as xcode_ninja_target_pattern='^whatever$' when
needed.

BTW, I'm surprised that ^.*$ is not the default value for
xcode_ninja_target_pattern. Maybe a bug should be created and referenced here.

https://codereview.chromium.org/421453003/diff/60001/test/mac/gyptest-action-...
File test/mac/gyptest-action-envvars.py (right):

https://codereview.chromium.org/421453003/diff/60001/test/mac/gyptest-action-...
test/mac/gyptest-action-envvars.py:18: # The ninja generator ignores SYMROOT,
while the xcode generator doesn't.
Same here.

https://codereview.chromium.org/421453003/diff/60001/test/mac/gyptest-global-...
File test/mac/gyptest-global-settings.py (right):

https://codereview.chromium.org/421453003/diff/60001/test/mac/gyptest-global-...
test/mac/gyptest-global-settings.py:19: # The ninja generator ignores SYMROOT,
while the xcode generator doesn't.
Same here.

https://codereview.chromium.org/421453003/diff/60001/test/mac/gyptest-ldflags.py
File test/mac/gyptest-ldflags.py (right):

https://codereview.chromium.org/421453003/diff/60001/test/mac/gyptest-ldflags...
test/mac/gyptest-ldflags.py:19: # The xcode-ninja generator fails to generate a
working wrapper project
Same here.

https://codereview.chromium.org/421453003/diff/60001/test/mac/gyptest-librari...
File test/mac/gyptest-libraries.py (right):

https://codereview.chromium.org/421453003/diff/60001/test/mac/gyptest-librari...
test/mac/gyptest-libraries.py:18: # The xcode-ninja generator fails to generate
a working wrapper project
Same here.

https://codereview.chromium.org/421453003/diff/60001/test/mac/gyptest-loadabl...
File test/mac/gyptest-loadable-module-bundle-product-extension.py (right):

https://codereview.chromium.org/421453003/diff/60001/test/mac/gyptest-loadabl...
test/mac/gyptest-loadable-module-bundle-product-extension.py:21: # The
xcode-ninja generator fails because it doesn't copy the extension
Same here.

https://codereview.chromium.org/421453003/diff/60001/test/ninja/action_depend...
File test/ninja/action_dependencies/gyptest-action-dependencies.py (right):

https://codereview.chromium.org/421453003/diff/60001/test/ninja/action_depend...
test/ninja/action_dependencies/gyptest-action-dependencies.py:24: if test.format
== 'xcode-ninja':
Please document why this test is ignored when using xcode-ninja.

https://codereview.chromium.org/421453003/diff/60001/test/ninja/chained-depen...
File test/ninja/chained-dependency/gyptest-chained-dependency.py (right):

https://codereview.chromium.org/421453003/diff/60001/test/ninja/chained-depen...
test/ninja/chained-dependency/gyptest-chained-dependency.py:21: if test.format
== 'xcode-ninja':
Same here.

https://codereview.chromium.org/421453003/diff/60001/test/rules/src/subdir4/b...
File test/rules/src/subdir4/build-asm.gyp (right):

https://codereview.chromium.org/421453003/diff/60001/test/rules/src/subdir4/b...
test/rules/src/subdir4/build-asm.gyp:40: 'bash', '-c', 'cp <(RULE_INPUT_PATH)
<@(_outputs)',
nit: can you fix the indentation to use spaces instead of tab?

https://codereview.chromium.org/421453003/diff/60001/test/same-target-name-di...
File test/same-target-name-different-directory/gyptest-all.py (right):

https://codereview.chromium.org/421453003/diff/60001/test/same-target-name-di...
test/same-target-name-different-directory/gyptest-all.py:15: # xcode-ninja fails
to generate a project due to id collisions
Same here.

https://codereview.chromium.org/421453003/diff/60001/test/sibling/gyptest-all.py
File test/sibling/gyptest-all.py (right):

https://codereview.chromium.org/421453003/diff/60001/test/sibling/gyptest-all...
test/sibling/gyptest-all.py:14: # The xcode-ninja generator fails to generate a
working wrapper project
Same here.

https://codereview.chromium.org/421453003/diff/60001/test/sibling/gyptest-rel...
File test/sibling/gyptest-relocate.py (right):

https://codereview.chromium.org/421453003/diff/60001/test/sibling/gyptest-rel...
test/sibling/gyptest-relocate.py:14: # The xcode-ninja generator fails to
generate a working wrapper project
Same here.

https://codereview.chromium.org/421453003/diff/60001/test/toplevel-dir/gyptes...
File test/toplevel-dir/gyptest-toplevel-dir.py (right):

https://codereview.chromium.org/421453003/diff/60001/test/toplevel-dir/gyptes...
test/toplevel-dir/gyptest-toplevel-dir.py:17: if test.format == 'xcode-ninja':
Please document.

[Tobias, 2014-10-02 18:18:53]

PTAL

[Tobias, 2014-10-02 18:19:20]

PTAL

https://codereview.chromium.org/421453003/diff/60001/test/build-option/gyptes...
File test/build-option/gyptest-build.py (right):

https://codereview.chromium.org/421453003/diff/60001/test/build-option/gyptes...
test/build-option/gyptest-build.py:23: # The xcode-ninja generator doesn't
support --build
I'm not sure. I've added a link to
https://code.google.com/p/gyp/issues/detail?id=453

On 2014/09/02 15:13:05, sdefresne wrote:
> Is this a bug or by design? If it is a bug, I think an issue should be created
> in the issue tracker, and a link to it should be added there, so that it get
> fixed.

https://codereview.chromium.org/421453003/diff/60001/test/compiler-override/g...
File test/compiler-override/gyptest-compiler-env.py (right):

https://codereview.chromium.org/421453003/diff/60001/test/compiler-override/g...
test/compiler-override/gyptest-compiler-env.py:113: # The xcode generator chokes
on the 'host' toolset.
I can't reproduce this one any more. I've removed the call to test_skip().

On 2014/09/02 15:13:05, sdefresne wrote:
> Is this a bug or by design?

https://codereview.chromium.org/421453003/diff/60001/test/compiler-override/g...
File test/compiler-override/gyptest-compiler-global-settings.py (right):

https://codereview.chromium.org/421453003/diff/60001/test/compiler-override/g...
test/compiler-override/gyptest-compiler-global-settings.py:46: # The xcode
generator chokes on the 'host' toolset.
On 2014/09/02 15:13:05, sdefresne wrote:
> Same question here.

This test is testing a feature that is apparently only available when using the
ninja and make generators (see line 22). I'm not sure if xcode-ninja should be
able to support it, as well. I've filed
https://code.google.com/p/gyp/issues/detail?id=454 and added a reference.

https://codereview.chromium.org/421453003/diff/60001/test/generator-output/gy...
File test/generator-output/gyptest-actions.py (right):

https://codereview.chromium.org/421453003/diff/60001/test/generator-output/gy...
test/generator-output/gyptest-actions.py:16: # Bug: xcode-ninja doesn't respect
--generator-output
This is the bug that triggered all of this ;)
https://code.google.com/p/gyp/issues/detail?id=442

This test will be enabled as part of https://codereview.chromium.org/421053005
I've added a comment to all generator-output tests.

On 2014/09/02 15:13:05, sdefresne wrote:
> Please file a bug (if it does not exists) and add reference here, something
> like:
> 
> # TODO(tobias.hahn): xcode-ninja doesn't respect --generator-output,
> http://crbug.com/xxxxxx
> 
> Or if this is by design, update instead the formats parameter of the
> TestGyp.TestGyp() call above to exclude xcode-ninja too.

https://codereview.chromium.org/421453003/diff/60001/test/lib/TestGyp.py
File test/lib/TestGyp.py (right):

https://codereview.chromium.org/421453003/diff/60001/test/lib/TestGyp.py#newc...
test/lib/TestGyp.py:291: run_args.extend(self.extra_args)
I share your feelings, I also feel a bit uneasy about adding this generator
option to every test.

My thinking behind adding it to the tests instead of the test runner is to make
the behavior of the xcode_ninja generator explicit, i.e. to avoid surprises
where your test works (because of an implicit target pattern of ^.*$), but not
when you run gyp yourself. I totally understand that there are good arguments
for either option, though, and I'd be happy to do whatever you prefer here.

The reason that ^.*$ is not the default generator target pattern is by design as
far as I understand (cf. the docstring of IsValidTargetForWrapper() in
xcode_ninja.py). Just building executable targets seems to be good enough in
'most' cases -- unless you're writing test cases that is ;)



On 2014/09/02 15:13:05, sdefresne wrote:
> I don't really like the fact that you add to change each and every test case
to
> add "-G xcode_ninja_target_pattern=^${something}$" as additional arguments,
with
> a default value of .* for something.
> 
> What about adding the following here:
> 
>   # Default xcode_ninja_target_pattern to ^.*$ to fix xcode-ninja tests
>   xcode_ninja_target_pattern = kw.pop('xcode_ninja_target_pattern', '^.*$')
>   run_args.extend(['-G', 'xcode_ninja_target_pattern=%s' %
> xcode_ninja_target_pattern)
> 
> Then you can omit the '-G', 'xcode_ninja_target_pattern=^whatever$' in most of
> the call of run_gyp, and pass it as xcode_ninja_target_pattern='^whatever$'
when
> needed.
> 
> BTW, I'm surprised that ^.*$ is not the default value for
> xcode_ninja_target_pattern. Maybe a bug should be created and referenced here.

https://codereview.chromium.org/421453003/diff/60001/test/mac/gyptest-action-...
File test/mac/gyptest-action-envvars.py (right):

https://codereview.chromium.org/421453003/diff/60001/test/mac/gyptest-action-...
test/mac/gyptest-action-envvars.py:18: # The ninja generator ignores SYMROOT,
while the xcode generator doesn't.
On 2014/09/02 15:13:05, sdefresne wrote:
> Same here.

Done (https://code.google.com/p/gyp/issues/detail?id=460)

https://codereview.chromium.org/421453003/diff/60001/test/mac/gyptest-global-...
File test/mac/gyptest-global-settings.py (right):

https://codereview.chromium.org/421453003/diff/60001/test/mac/gyptest-global-...
test/mac/gyptest-global-settings.py:19: # The ninja generator ignores SYMROOT,
while the xcode generator doesn't.
On 2014/09/02 15:13:05, sdefresne wrote:
> Same here.

Done.

https://codereview.chromium.org/421453003/diff/60001/test/mac/gyptest-ldflags.py
File test/mac/gyptest-ldflags.py (right):

https://codereview.chromium.org/421453003/diff/60001/test/mac/gyptest-ldflags...
test/mac/gyptest-ldflags.py:19: # The xcode-ninja generator fails to generate a
working wrapper project
On 2014/09/02 15:13:05, sdefresne wrote:
> Same here.

Done.

https://codereview.chromium.org/421453003/diff/60001/test/mac/gyptest-librari...
File test/mac/gyptest-libraries.py (right):

https://codereview.chromium.org/421453003/diff/60001/test/mac/gyptest-librari...
test/mac/gyptest-libraries.py:18: # The xcode-ninja generator fails to generate
a working wrapper project
On 2014/09/02 15:13:05, sdefresne wrote:
> Same here.

Done.

https://codereview.chromium.org/421453003/diff/60001/test/mac/gyptest-loadabl...
File test/mac/gyptest-loadable-module-bundle-product-extension.py (right):

https://codereview.chromium.org/421453003/diff/60001/test/mac/gyptest-loadabl...
test/mac/gyptest-loadable-module-bundle-product-extension.py:21: # The
xcode-ninja generator fails because it doesn't copy the extension
On 2014/09/02 15:13:05, sdefresne wrote:
> Same here.

Done.

https://codereview.chromium.org/421453003/diff/60001/test/ninja/action_depend...
File test/ninja/action_dependencies/gyptest-action-dependencies.py (right):

https://codereview.chromium.org/421453003/diff/60001/test/ninja/action_depend...
test/ninja/action_dependencies/gyptest-action-dependencies.py:24: if test.format
== 'xcode-ninja':
On 2014/09/02 15:13:05, sdefresne wrote:
> Please document why this test is ignored when using xcode-ninja.

As explained in the comment above "NOTE(piman) [...]", this test relies on being
able to build a specific object file. This is a feature that only some
generators support (not xcode-ninja). I've added a comment explaining this.

https://codereview.chromium.org/421453003/diff/60001/test/ninja/chained-depen...
File test/ninja/chained-dependency/gyptest-chained-dependency.py (right):

https://codereview.chromium.org/421453003/diff/60001/test/ninja/chained-depen...
test/ninja/chained-dependency/gyptest-chained-dependency.py:21: if test.format
== 'xcode-ninja':
On 2014/09/02 15:13:06, sdefresne wrote:
> Same here.

Done.

https://codereview.chromium.org/421453003/diff/60001/test/rules/src/subdir4/b...
File test/rules/src/subdir4/build-asm.gyp (right):

https://codereview.chromium.org/421453003/diff/60001/test/rules/src/subdir4/b...
test/rules/src/subdir4/build-asm.gyp:40: 'bash', '-c', 'cp <(RULE_INPUT_PATH)
<@(_outputs)',
On 2014/09/02 15:13:06, sdefresne wrote:
> nit: can you fix the indentation to use spaces instead of tab?

Oops ;) Done.

https://codereview.chromium.org/421453003/diff/60001/test/same-target-name-di...
File test/same-target-name-different-directory/gyptest-all.py (right):

https://codereview.chromium.org/421453003/diff/60001/test/same-target-name-di...
test/same-target-name-different-directory/gyptest-all.py:15: # xcode-ninja fails
to generate a project due to id collisions
On 2014/09/02 15:13:06, sdefresne wrote:
> Same here.

Done.

https://codereview.chromium.org/421453003/diff/60001/test/sibling/gyptest-all.py
File test/sibling/gyptest-all.py (right):

https://codereview.chromium.org/421453003/diff/60001/test/sibling/gyptest-all...
test/sibling/gyptest-all.py:14: # The xcode-ninja generator fails to generate a
working wrapper project
On 2014/09/02 15:13:06, sdefresne wrote:
> Same here.

Done.

https://codereview.chromium.org/421453003/diff/60001/test/sibling/gyptest-rel...
File test/sibling/gyptest-relocate.py (right):

https://codereview.chromium.org/421453003/diff/60001/test/sibling/gyptest-rel...
test/sibling/gyptest-relocate.py:14: # The xcode-ninja generator fails to
generate a working wrapper project
On 2014/09/02 15:13:06, sdefresne wrote:
> Same here.

Done.

https://codereview.chromium.org/421453003/diff/60001/test/toplevel-dir/gyptes...
File test/toplevel-dir/gyptest-toplevel-dir.py (right):

https://codereview.chromium.org/421453003/diff/60001/test/toplevel-dir/gyptes...
test/toplevel-dir/gyptest-toplevel-dir.py:17: if test.format == 'xcode-ninja':
On 2014/09/02 15:13:06, sdefresne wrote:
> Please document.

This test actually was working by chance only. The first parameter to build is
the gyp file, not the target. I fixed this, and the test passes.

[sdefresne, 2014-12-03 16:12:34]

lgtm

[Tobias, 2014-12-08 19:25:35]

Patchset #9 (id:160001) has been deleted

[sdefresne, 2014-12-23 14:14:46]

Committed patchset #10 (id:200001) manually as 2017 (presubmit successful).