iOS: Enable modules_unittests and common_audio_unittests

iOS: Enable modules_unittests and common_audio_unittests

Instead of excluding the whole test binaries, only exclude the parts that cause the
compilation to fail for modules_unittests and common_audio_unittests.

BUG=webrtc:4752, webrtc:4755, webrtc:5544
TESTED=Successful build with:
GYP_DEFINES='OS=ios target_arch=x64' webrtc/build/gyp_webrtc
ninja -C out/Debug-iphonesimulator modules_unittests common_audio_unittests
NOTRY=True

Committed: https://crrev.com/9ac4df1ba66d39c3621cfb2e8ed08ae39658b793
Cr-Commit-Position: refs/heads/master@{#11675}

[kjellander_webrtc, 2016-02-15 10:14:39]

Description was changed from

==========
iOS: Enable modules_unittests and common_audio_unittests

Instead of excluding the tests, exclude only the parts that cause the
linking problems for modules_unittests.
common_audio_unittests seems to have been fixed by other changes, since
it could be enabled right away.

BUG=webrtc:4755
TESTED=Successful build with:
GYP_DEFINES='OS=ios target_arch=x64' webrtc/build/gyp_webrtc
ninja -C out/Debug-iphonesimulator modules_unittests common_audio_unittests
==========

to

==========
iOS: Enable modules_unittests and common_audio_unittests

Instead of excluding the tests, exclude only the parts that cause the
linking problems for modules_unittests.
common_audio_unittests seems to have been fixed by other changes, since
it could be enabled right away.

BUG=webrtc:4755
TESTED=Successful build with:
GYP_DEFINES='OS=ios target_arch=x64' webrtc/build/gyp_webrtc
ninja -C out/Debug-iphonesimulator modules_unittests common_audio_unittests
==========

[kjellander_webrtc, 2016-02-15 10:14:39]

kjellander@webrtc.org changed reviewers:
+ sergeyu@chromium.org

[kjellander_webrtc, 2016-02-15 10:32:39]

Patchset #1 (id:1) has been deleted

[kjellander_webrtc, 2016-02-15 12:25:50]

Patchset #1 (id:20001) has been deleted

[kjellander_webrtc, 2016-02-15 12:28:02]

kjellander@webrtc.org changed reviewers:
+ tina.legrand@webrtc.org

[kjellander_webrtc, 2016-02-15 12:28:29]

Description was changed from

==========
iOS: Enable modules_unittests and common_audio_unittests

Instead of excluding the tests, exclude only the parts that cause the
linking problems for modules_unittests.
common_audio_unittests seems to have been fixed by other changes, since
it could be enabled right away.

BUG=webrtc:4755
TESTED=Successful build with:
GYP_DEFINES='OS=ios target_arch=x64' webrtc/build/gyp_webrtc
ninja -C out/Debug-iphonesimulator modules_unittests common_audio_unittests
==========

to

==========
iOS: Enable modules_unittests and common_audio_unittests

Instead of excluding the tests, exclude only the parts that cause the
linking problems for modules_unittests.
common_audio_unittests seems to have been fixed by other changes, since
it could be enabled right away.

BUG=webrtc:4752, webrtc:4755
TESTED=Successful build with:
GYP_DEFINES='OS=ios target_arch=x64' webrtc/build/gyp_webrtc
ninja -C out/Debug-iphonesimulator modules_unittests common_audio_unittests
==========

[kjellander_webrtc, 2016-02-15 12:28:50]

kjellander@webrtc.org changed reviewers:
+ kwiberg@webrtc.org
- tina.legrand@webrtc.org

[kjellander_webrtc, 2016-02-15 12:34:27]

Description was changed from

==========
iOS: Enable modules_unittests and common_audio_unittests

Instead of excluding the tests, exclude only the parts that cause the
linking problems for modules_unittests.
common_audio_unittests seems to have been fixed by other changes, since
it could be enabled right away.

BUG=webrtc:4752, webrtc:4755
TESTED=Successful build with:
GYP_DEFINES='OS=ios target_arch=x64' webrtc/build/gyp_webrtc
ninja -C out/Debug-iphonesimulator modules_unittests common_audio_unittests
==========

to

==========
iOS: Enable modules_unittests and common_audio_unittests

Instead of excluding the tests, exclude only the parts that cause the
linking problems for modules_unittests.
common_audio_unittests seems to have been fixed by other changes, since
it could be enabled right away.

BUG=webrtc:4752, webrtc:4755
TESTED=Successful build with:
GYP_DEFINES='OS=ios target_arch=x64' webrtc/build/gyp_webrtc
ninja -C out/Debug-iphonesimulator modules_unittests common_audio_unittests
NOTRY=True
==========

[kjellander_webrtc, 2016-02-15 12:35:09]

Description was changed from

==========
iOS: Enable modules_unittests and common_audio_unittests

Instead of excluding the tests, exclude only the parts that cause the
linking problems for modules_unittests.
common_audio_unittests seems to have been fixed by other changes, since
it could be enabled right away.

BUG=webrtc:4752, webrtc:4755
TESTED=Successful build with:
GYP_DEFINES='OS=ios target_arch=x64' webrtc/build/gyp_webrtc
ninja -C out/Debug-iphonesimulator modules_unittests common_audio_unittests
NOTRY=True
==========

to

==========
iOS: Enable modules_unittests and common_audio_unittests

Instead of excluding the whole test binaries, only exclude the parts that cause
the
compilation to fail for modules_unittests and common_audio_unittests.

BUG=webrtc:4752, webrtc:4755
TESTED=Successful build with:
GYP_DEFINES='OS=ios target_arch=x64' webrtc/build/gyp_webrtc
ninja -C out/Debug-iphonesimulator modules_unittests common_audio_unittests
NOTRY=True
==========

[kjellander_webrtc, 2016-02-15 20:42:58]

Patchset #1 (id:40001) has been deleted

[kjellander_webrtc, 2016-02-15 20:43:04]

Patchset #1 (id:60001) has been deleted

[kjellander_webrtc, 2016-02-15 21:05:04]

sergeyu: desktop capture changes.
kwiberg: common_audio.gyp + iSAC changes in modules.gyp

kwiberg: do you know how we can get the iSAC stuff solved for iOS, so it builds
for all architectures, even for iOS?

https://codereview.webrtc.org/1698033002/diff/80001/webrtc/modules/modules.gyp
File webrtc/modules/modules.gyp (right):

https://codereview.webrtc.org/1698033002/diff/80001/webrtc/modules/modules.gy...
webrtc/modules/modules.gyp:113: 'target_name': 'modules_unittests',
The diff is useless here: what I've done is:
1. move the target out of the condition so it's built for iOS.
2. move the desktop_capture dependency + tests into the
desktop_capture_supported==1 condition
3. move the iSAC fixed tests that fail to link on iOS ia32/x64/arm64 into a new
condition (at the end of the target).

See comments below for more details. No other modifications are made to the
target.

https://codereview.webrtc.org/1698033002/diff/80001/webrtc/modules/modules.gy...
webrtc/modules/modules.gyp:382: 'dependencies': [
I moved the desktop_capture dependency + test sources into this condition,
instead of subtracting some tests when desktop_capture_supported==0 (which I
believe may have been outdated).

I changed the condition from 
==0 and excluding sources, to
==1 and including the sources + the 'desktop_capture' library.
to improve readability.

NOTICE: This means a few tests that used to run on Android no longer run. They
are:
desktop_capture/desktop_region_unittest.cc
desktop_capture/differ_block_unittest.cc
desktop_capture/differ_unittest.cc

I assume that's fine, considering Android is not really a platform where desktop
capture is used/supported, or is it?

https://codereview.webrtc.org/1698033002/diff/80001/webrtc/modules/modules.gy...
webrtc/modules/modules.gyp:474: 'audio_coding/codecs/isac/unittest.cc',
These 5 are the sources that are moved out from the large list, since they don't
link on iOS for x86/x64/arm64. 
They're still kept for non-iOS platforms and iOS ARM (even though we don't run
tests for that configuration yet).

[Sergey Ulanov, 2016-02-16 18:52:14]

https://codereview.chromium.org/1698033002/diff/80001/webrtc/modules/modules.gyp
File webrtc/modules/modules.gyp (right):

https://codereview.chromium.org/1698033002/diff/80001/webrtc/modules/modules....
webrtc/modules/modules.gyp:382: 'dependencies': [
On 2016/02/15 21:05:04, kjellander (webrtc) wrote:
> I moved the desktop_capture dependency + test sources into this condition,
> instead of subtracting some tests when desktop_capture_supported==0 (which I
> believe may have been outdated).
> 
> I changed the condition from 
> ==0 and excluding sources, to
> ==1 and including the sources + the 'desktop_capture' library.
> to improve readability.
> 
> NOTICE: This means a few tests that used to run on Android no longer run. They
> are:
> desktop_capture/desktop_region_unittest.cc
> desktop_capture/differ_block_unittest.cc
> desktop_capture/differ_unittest.cc
> 
> I assume that's fine, considering Android is not really a platform where
desktop
> capture is used/supported, or is it?

DesktopRegion is used by the remoting client on Android. Beside DesktopRegion
the stuff in desktop_geometry.h is also used by the remoting client code and
should still be enabled on Android.

[kjellander_webrtc, 2016-02-16 19:37:37]

Thanks for the quick reply. I restored the Android tests in PS#2. 
PTAL

https://codereview.webrtc.org/1698033002/diff/80001/webrtc/modules/modules.gyp
File webrtc/modules/modules.gyp (right):

https://codereview.webrtc.org/1698033002/diff/80001/webrtc/modules/modules.gy...
webrtc/modules/modules.gyp:382: 'dependencies': [
On 2016/02/16 18:52:13, Sergey Ulanov wrote:
> On 2016/02/15 21:05:04, kjellander (webrtc) wrote:
> > I moved the desktop_capture dependency + test sources into this condition,
> > instead of subtracting some tests when desktop_capture_supported==0 (which I
> > believe may have been outdated).
> > 
> > I changed the condition from 
> > ==0 and excluding sources, to
> > ==1 and including the sources + the 'desktop_capture' library.
> > to improve readability.
> > 
> > NOTICE: This means a few tests that used to run on Android no longer run.
They
> > are:
> > desktop_capture/desktop_region_unittest.cc
> > desktop_capture/differ_block_unittest.cc
> > desktop_capture/differ_unittest.cc
> > 
> > I assume that's fine, considering Android is not really a platform where
> desktop
> > capture is used/supported, or is it?
> 
> DesktopRegion is used by the remoting client on Android. Beside DesktopRegion
> the stuff in desktop_geometry.h is also used by the remoting client code and
> should still be enabled on Android.

Alright, I restored them in the next patch set.

[kwiberg-webrtc, 2016-02-17 10:01:03]

lgtm, but see comment

https://codereview.webrtc.org/1698033002/diff/100001/webrtc/modules/modules.gyp
File webrtc/modules/modules.gyp (right):

https://codereview.webrtc.org/1698033002/diff/100001/webrtc/modules/modules.g...
webrtc/modules/modules.gyp:476: ['OS!="ios" or target_arch=="arm"', {
Hmm. The comment is equivalent to the condition only if ia32,x64, arm, and arm64
is the complete set of target archs.

Also, you changed this to exclude arm64. Why?

[kwiberg-webrtc, 2016-02-17 10:04:02]

On 2016/02/15 21:05:04, kjellander (webrtc) wrote:
> kwiberg: do you know how we can get the iSAC stuff solved for iOS, so it
builds
> for all architectures, even for iOS?

No, I don't know what the problem is. (Though admittedly, I haven't tried to
find out.)

It might make sense to split it out into its own bug now, though.

[kjellander_webrtc, 2016-02-17 20:16:27]

Patchset #3 (id:120001) has been deleted

[kjellander_webrtc, 2016-02-17 20:42:16]

Patchset #3 (id:140001) has been deleted

[kjellander_webrtc, 2016-02-17 20:46:10]

Patchset #3 (id:160001) has been deleted

[kjellander_webrtc, 2016-02-17 20:51:05]

Patchset #3 (id:180001) has been deleted

[kjellander_webrtc, 2016-02-17 20:54:25]

Patchset #3 (id:200001) has been deleted

[kjellander_webrtc, 2016-02-17 20:55:34]

Description was changed from

==========
iOS: Enable modules_unittests and common_audio_unittests

Instead of excluding the whole test binaries, only exclude the parts that cause
the
compilation to fail for modules_unittests and common_audio_unittests.

BUG=webrtc:4752, webrtc:4755
TESTED=Successful build with:
GYP_DEFINES='OS=ios target_arch=x64' webrtc/build/gyp_webrtc
ninja -C out/Debug-iphonesimulator modules_unittests common_audio_unittests
NOTRY=True
==========

to

==========
iOS: Enable modules_unittests and common_audio_unittests

Instead of excluding the whole test binaries, only exclude the parts that cause
the
compilation to fail for modules_unittests and common_audio_unittests.

BUG=webrtc:4752, webrtc:4755, webrtc:5544
TESTED=Successful build with:
GYP_DEFINES='OS=ios target_arch=x64' webrtc/build/gyp_webrtc
ninja -C out/Debug-iphonesimulator modules_unittests common_audio_unittests
NOTRY=True
==========

[kjellander_webrtc, 2016-02-17 21:03:45]

Please take a look at PS#3+4 combined (saved some trybot runs there).

I did some more testing locally and found the mistakes I initially made.
modules_unittests will need more work for all tests to pass, but that can be
done in a follow-up CL (it's just running in
http://build.chromium.org/p/client.webrtc.fyi/waterfall now anyway).

https://codereview.webrtc.org/1698033002/diff/100001/webrtc/modules/modules.gyp
File webrtc/modules/modules.gyp (right):

https://codereview.webrtc.org/1698033002/diff/100001/webrtc/modules/modules.g...
webrtc/modules/modules.gyp:476: ['OS!="ios" or target_arch=="arm"', {
On 2016/02/17 10:01:03, kwiberg-webrtc wrote:
> Hmm. The comment is equivalent to the condition only if ia32,x64, arm, and
arm64
> is the complete set of target archs.
> 
> Also, you changed this to exclude arm64. Why?

Good catch! You're right; this code was previously enabled for arm64 so I must
have been confused somehow. I did some more testing and it appears it actually
compiles for ia32 and x64 as well so I must have been building the wrong out
directory (arm,arm64 = out/{Debug,Release}-iphoneos and ia32,x64 = arm,arm64 =
out/{Debug,Release}-iphonesimulator). You get odd link errors when trying to
build the regular out/{Debug,Release} configs (desktop) so that must have been
what happened here.

Lesson learned: always include both environment, build command and compile
errors in the bugs.

[kwiberg-webrtc, 2016-02-18 09:54:50]

lgtm

https://codereview.webrtc.org/1698033002/diff/240001/webrtc/modules/modules.gyp
File webrtc/modules/modules.gyp (right):

https://codereview.webrtc.org/1698033002/diff/240001/webrtc/modules/modules.g...
webrtc/modules/modules.gyp:461: # This needs to be kept in sync with
modules_unittests.isolate.
Bug waiting to happen. :-)

[kjellander_webrtc, 2016-02-18 13:34:39]

Friendly ping, Sergey.

https://codereview.webrtc.org/1698033002/diff/240001/webrtc/modules/modules.gyp
File webrtc/modules/modules.gyp (right):

https://codereview.webrtc.org/1698033002/diff/240001/webrtc/modules/modules.g...
webrtc/modules/modules.gyp:461: # This needs to be kept in sync with
modules_unittests.isolate.
On 2016/02/18 09:54:50, kwiberg-webrtc wrote:
> Bug waiting to happen. :-)

Yeah, I've reached out to the iOS folks and asked if there's a way to use the
.isolate files.
At least things will break on trybots when one or the other gets out of sync,
once we've moved the iOS simulator bots to the main waterfall.

[Sergey Ulanov, 2016-02-18 18:41:52]

lgtm

[kjellander_webrtc, 2016-02-18 21:10:47]

The CQ bit was checked by kjellander@webrtc.org

[commit-bot: I haz the power, 2016-02-18 21:10:55]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1698033002/240001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1698033002/240001

[commit-bot: I haz the power, 2016-02-18 21:15:13]

Description was changed from

==========
iOS: Enable modules_unittests and common_audio_unittests

Instead of excluding the whole test binaries, only exclude the parts that cause
the
compilation to fail for modules_unittests and common_audio_unittests.

BUG=webrtc:4752, webrtc:4755, webrtc:5544
TESTED=Successful build with:
GYP_DEFINES='OS=ios target_arch=x64' webrtc/build/gyp_webrtc
ninja -C out/Debug-iphonesimulator modules_unittests common_audio_unittests
NOTRY=True
==========

to

==========
iOS: Enable modules_unittests and common_audio_unittests

Instead of excluding the whole test binaries, only exclude the parts that cause
the
compilation to fail for modules_unittests and common_audio_unittests.

BUG=webrtc:4752, webrtc:4755, webrtc:5544
TESTED=Successful build with:
GYP_DEFINES='OS=ios target_arch=x64' webrtc/build/gyp_webrtc
ninja -C out/Debug-iphonesimulator modules_unittests common_audio_unittests
NOTRY=True
==========

[commit-bot: I haz the power, 2016-02-18 21:15:14]

Committed patchset #4 (id:240001)

[commit-bot: I haz the power, 2016-02-18 21:15:23]

Description was changed from

==========
iOS: Enable modules_unittests and common_audio_unittests

Instead of excluding the whole test binaries, only exclude the parts that cause
the
compilation to fail for modules_unittests and common_audio_unittests.

BUG=webrtc:4752, webrtc:4755, webrtc:5544
TESTED=Successful build with:
GYP_DEFINES='OS=ios target_arch=x64' webrtc/build/gyp_webrtc
ninja -C out/Debug-iphonesimulator modules_unittests common_audio_unittests
NOTRY=True
==========

to

==========
iOS: Enable modules_unittests and common_audio_unittests

Instead of excluding the whole test binaries, only exclude the parts that cause
the
compilation to fail for modules_unittests and common_audio_unittests.

BUG=webrtc:4752, webrtc:4755, webrtc:5544
TESTED=Successful build with:
GYP_DEFINES='OS=ios target_arch=x64' webrtc/build/gyp_webrtc
ninja -C out/Debug-iphonesimulator modules_unittests common_audio_unittests
NOTRY=True

Committed: https://crrev.com/9ac4df1ba66d39c3621cfb2e8ed08ae39658b793
Cr-Commit-Position: refs/heads/master@{#11675}
==========

[commit-bot: I haz the power, 2016-02-18 21:15:24]

Patchset 4 (id:??) landed as
https://crrev.com/9ac4df1ba66d39c3621cfb2e8ed08ae39658b793
Cr-Commit-Position: refs/heads/master@{#11675}