Add build targets for Opus tests.

Add build targets for Opus tests.

test_opus_api, test_opus_encode, test_opus_decode, and test_opus_padding
can be run directly, with no command-line options, and they print out
"OK" during the test and an overall status message such as
"All API tests passed." or "Tests completed successfully." at the end.

The celt component has several unit tests. I didn't add build targets
for them because some of them don't print any output, so you'd need a
test harness script to examine their exit status.

R=henrika@chromium.org,sergeyu@chromium.org
BUG=452984
Committed: https://crrev.com/bfcc6156a8f2107ecd70445b08e60299737caffa
Cr-Commit-Position: refs/heads/master@{#313964}

[wtc, 2015-01-29 03:09:19]

Please review.

Is there a way to share code between these build targets? I am not that good at
GYP and GN :-(

[Sergey Ulanov, 2015-01-29 18:12:21]

https://codereview.chromium.org/890473002/diff/1/third_party/opus/BUILD.gn
File third_party/opus/BUILD.gn (right):

https://codereview.chromium.org/890473002/diff/1/third_party/opus/BUILD.gn#ne...
third_party/opus/BUILD.gn:234: include_dirs = [
These settings are duplicated between test targets. is it possible to define
them once and use for all test targets?

https://codereview.chromium.org/890473002/diff/1/third_party/opus/opus.gyp
File third_party/opus/opus.gyp (right):

https://codereview.chromium.org/890473002/diff/1/third_party/opus/opus.gyp#ne...
third_party/opus/opus.gyp:218: 'defines': [
maybe put this in target_defaults?
Same for android and clang flags below

[brettw, 2015-01-29 20:25:29]

brettw@chromium.org changed reviewers:
+ brettw@chromium.org

[brettw, 2015-01-29 20:25:30]

lgtm

https://codereview.chromium.org/890473002/diff/40001/third_party/opus/BUILD.gn
File third_party/opus/BUILD.gn (right):

https://codereview.chromium.org/890473002/diff/40001/third_party/opus/BUILD.g...
third_party/opus/BUILD.gn:219: executable("test_opus_api") {
Can you do "test" instead of "executable"? You will have to add
  import("//testing/test.gni")
to the top of this file. This will set some testing-related defaults and make it
work on Android.

[Sergey Ulanov, 2015-01-29 21:00:38]

lgtm

[wtc, 2015-01-29 21:11:33]

Please review patch set 4.

https://codereview.chromium.org/890473002/diff/1/third_party/opus/BUILD.gn
File third_party/opus/BUILD.gn (right):

https://codereview.chromium.org/890473002/diff/1/third_party/opus/BUILD.gn#ne...
third_party/opus/BUILD.gn:234: include_dirs = [

On 2015/01/29 18:12:20, Sergey Ulanov wrote:
> These settings are duplicated between test targets. is it possible to define
> them once and use for all test targets?

Done. I added a GN config on Brett's suggestion.

https://codereview.chromium.org/890473002/diff/1/third_party/opus/opus.gyp
File third_party/opus/opus.gyp (right):

https://codereview.chromium.org/890473002/diff/1/third_party/opus/opus.gyp#ne...
third_party/opus/opus.gyp:218: 'defines': [

On 2015/01/29 18:12:20, Sergey Ulanov wrote:
> maybe put this in target_defaults?
> Same for android and clang flags below

Done. Thanks a lot for the suggestion.

https://codereview.chromium.org/890473002/diff/40001/third_party/opus/BUILD.gn
File third_party/opus/BUILD.gn (right):

https://codereview.chromium.org/890473002/diff/40001/third_party/opus/BUILD.g...
third_party/opus/BUILD.gn:219: executable("test_opus_api") {

On 2015/01/29 20:25:30, brettw wrote:
> Can you do "test" instead of "executable"? You will have to add
>   import("//testing/test.gni")
> to the top of this file. This will set some testing-related defaults and make
it
> work on Android.

Done. I have two questions.

1. I didn't make this change to the opus_compare and opus_demo targets because
they are more like utility programs. Is that OK?

2. These Opus test programs don't use gtest. Using "test" won't make them depend
on gtest, right?

[brettw, 2015-01-30 19:02:30]

LGTM

https://codereview.chromium.org/890473002/diff/40001/third_party/opus/BUILD.gn
File third_party/opus/BUILD.gn (right):

https://codereview.chromium.org/890473002/diff/40001/third_party/opus/BUILD.g...
third_party/opus/BUILD.gn:219: executable("test_opus_api") {
> 2. These Opus test programs don't use gtest. Using "test" won't make them
depend
> on gtest, right?

Right.

[wtc, 2015-01-30 19:32:23]

https://codereview.chromium.org/890473002/diff/60001/third_party/opus/opus.gyp
File third_party/opus/opus.gyp (right):

https://codereview.chromium.org/890473002/diff/60001/third_party/opus/opus.gy...
third_party/opus/opus.gyp:50: 'link_settings': {

Just wanted to explain why I had to remove 'link_settings'.

When placed inside a 'target_conditions' section, 'link_settings' causes a GYP
error on the Android builtbots:

________ running '/usr/bin/python src/build/gyp_chromium' in
'/b/build/slave/android_compile_rel/build'
Updating projects from gyp files...
gyp: link_settings not allowed in the Debug configuration, found in target
/b/build/slave/android_compile_rel/build/src/third_party/opus/opus.gyp:test_opus_api#target
Error: Command /usr/bin/python src/build/gyp_chromium returned non-zero exit
status 1 in /b/build/slave/android_compile_rel/build
Hook '/usr/bin/python src/build/gyp_chromium' took 20.36 secs

I consulted Mark Mentovai. I don't understand his explanation of why GYP
disallows this, but he gave me his blessing for using 'libraries' alone, instead
of inside 'link_settings'.

[wtc, 2015-01-30 19:32:41]

The CQ bit was checked by wtc@chromium.org

[commit-bot: I haz the power, 2015-01-30 19:33:29]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/890473002/80001

[commit-bot: I haz the power, 2015-01-30 19:37:54]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-01-30 19:39:25]

Patchset 5 (id:??) landed as
https://crrev.com/bfcc6156a8f2107ecd70445b08e60299737caffa
Cr-Commit-Position: refs/heads/master@{#313964}

[please use gerrit instead, 2015-01-30 19:57:45]

A revert of this CL (patchset #5 id:80001) has been created in
https://codereview.chromium.org/888963002/ by rouslan@chromium.org.

The reason for reverting is: Broke Mac GN (dbg) bot.

[169/1394] STAMP
obj/third_party/cacheinvalidation/cacheinvalidation_unittests.inputdeps.stamp
FAILED: ../../third_party/llvm-build/Release+Asserts/bin/clang -MMD -MF
obj/third_party/opus/src/tests/test_opus_decode.test_opus_decode.o.d
-DCHROMIUM_BUILD -DV8_DEPRECATION_WARNINGS -DCLD_VERSION=2
-DENABLE_NOTIFICATIONS -DENABLE_PEPPER_CDMS -DENABLE_PLUGINS=1
-DENABLE_PRINTING=1 -DENABLE_BASIC_PRINTING=1 -DENABLE_PRINT_PREVIEW=1
-DENABLE_SPELLCHECK=1 -DUSE_OPENSSL=1 -DNO_TCMALLOC -DDISABLE_NACL
-DENABLE_EXTENSIONS=1 -DENABLE_CONFIGURATION_POLICY -DENABLE_TASK_MANAGER=1
-DENABLE_THEMES=1 -DENABLE_CAPTIVE_PORTAL_DETECTION=1 -DENABLE_SESSION_SERVICE=1
-DENABLE_PLUGIN_INSTALLATION=1 -DENABLE_APP_LIST=1 -DENABLE_SETTINGS_APP=1
-DENABLE_SUPERVISED_USERS=1 -DENABLE_SERVICE_DISCOVERY=1
-DENABLE_AUTOFILL_DIALOG=1 -DENABLE_WIFI_BOOTSTRAPPING=1 -DENABLE_REMOTING=1
-DENABLE_GOOGLE_NOW=1 -DENABLE_ONE_CLICK_SIGNIN -DENABLE_HIDPI=1
-DV8_USE_EXTERNAL_STARTUP_DATA -DCR_CLANG_REVISION=223108 -D_DEBUG
-DDYNAMIC_ANNOTATIONS_ENABLED=1 -DWTF_USE_DYNAMIC_ANNOTATIONS=1 -I../.. -Igen
-I../../third_party/opus/src/celt -I../../third_party/opus/src/silk
-I../../third_party/opus/src/include -fno-strict-aliasing -fstack-protector-all
-arch x86_64 -fcolor-diagnostics -Wendif-labels -Werror
-Wno-missing-field-initializers -Wno-unused-parameter -Wnewline-eof
-Wno-c++11-narrowing -Wno-char-subscripts -Wno-covered-switch-default
-Wno-deprecated-register -Wno-unneeded-internal-declaration
-Wno-reserved-user-defined-literal -Wno-inconsistent-missing-override
-fvisibility=hidden -isysroot
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.6.sdk
-mmacosx-version-min=10.6 -Xclang -load -Xclang
../../third_party/llvm-build/Release+Asserts/lib/libFindBadConstructs.dylib
-Xclang -add-plugin -Xclang find-bad-constructs -Wheader-hygiene
-Wstring-conversion -O0 -g2 -Wno-absolute-value -std=c99 -c
../../third_party/opus/src/tests/test_opus_decode.c -o
obj/third_party/opus/src/tests/test_opus_decode.test_opus_decode.o
../../third_party/opus/src/tests/test_opus_decode.c:138:64: error: null passed
to a callee that requires a non-null argument [-Werror,-Wnonnull]
         out_samples = opus_decode(dec[t], packet, 0, 0, 0, fec);
                                                      ~        ^
1 error generated.
ninja: build stopped: subcommand failed.

http://build.chromium.org/p/chromium.mac/builders/Mac%20GN%20%28dbg%29/builds....

[brettw, 2015-01-30 21:33:07]

:( Sorry about this wtc. Next time you can manually trigger the GN Mac bot. I've
been wanting to get this by default but there's not enough trybot hardware or
something.

[wtc, 2015-01-30 22:00:38]

Brett: no worries. I didn't realize there are bots that build every target.

Is it possible to exclude a target from the 'all' target in GN? I am not
sure if this is the right solution, but am curious about the answer.

The test program in question passes a null argument and verifies that the
function fails. I will try to figure out how to disable the
   __attribute__ ((__nonnull__(_x)))
on the function declaration when compiling that test program.

Do you know where the -Wnonnull flag comes from? It is not in the clang
command line that Rouslan pasted in comment 13.

[brettw, 2015-01-30 22:50:46]

I think it's automatically included when using -Werror.