Make isac_fix_test use gtest (in a hacky way)

Make isac_fix_test use gtest (in a hacky way)

This test is the only remaining one that does not use gtest and that's
blocking some infra cleanup tasks. Ideally this test would use
webrtc/rtc_base/flags.h but that's a lot of unnecessary work.

This also replaces some exit() status codes - the logic behind this is
if you get incorrectly specified command line arguments, exit(1) is
invoked for a failure, because it's not a test failure, and if flag
parsing was done properly, it would not be a gtest failure anyway.

BUG=webrtc:7568
Review-Url: https://codereview.webrtc.org/3000033002
Cr-Commit-Position: refs/heads/master@{#19388}
Committed: https://chromium.googlesource.com/external/webrtc/+/168576be1eac18c781de809ddedcd0b78e86daf3

[oprypin_webrtc, 2017-08-16 13:59:02]

Description was changed from

==========
Make isac_fix_test use gtest (in a hacky way)

This test is the only remaining one that does not use gtest and that's
blocking some infra cleanup tasks. Ideally this test would use
webrtc/rtc_base/flags.h but that's a lot of unnecessary work.

BUG=webrtc:7568
==========

to

==========
Make isac_fix_test use gtest (in a hacky way)

This test is the only remaining one that does not use gtest and that's
blocking some infra cleanup tasks. Ideally this test would use
webrtc/rtc_base/flags.h but that's a lot of unnecessary work.

BUG=webrtc:7568
==========

[oprypin_webrtc, 2017-08-16 13:59:02]

oprypin@webrtc.org changed reviewers:
+ henrik.lundin@webrtc.org, kjellander@webrtc.org

[kjellander_webrtc, 2017-08-16 14:53:14]

lgtm

[oprypin_webrtc, 2017-08-17 09:47:04]

oprypin@webrtc.org changed reviewers:
+ minyue@webrtc.org

[oprypin_webrtc, 2017-08-17 09:47:16]

friendly ping - needs owners' review

[kwiberg-webrtc, 2017-08-17 09:53:28]

kwiberg@webrtc.org changed reviewers:
+ kwiberg@webrtc.org

[kwiberg-webrtc, 2017-08-17 09:53:29]

There are a whole bunch of exit() calls---don't you need to replace them too
with something gtesty?

[oprypin_webrtc, 2017-08-17 11:02:20]

On 2017/08/17 09:53:29, kwiberg-webrtc wrote:
> There are a whole bunch of exit() calls---don't you need to replace them too
> with something gtesty?

Well it's not strictly necessary, the return status code would cause a failure
anyway.
I avoided doing this initially because this test needs a ton more work for a
proper refactor.
Done now.

[oprypin_webrtc, 2017-08-17 11:04:12]

Description was changed from

==========
Make isac_fix_test use gtest (in a hacky way)

This test is the only remaining one that does not use gtest and that's
blocking some infra cleanup tasks. Ideally this test would use
webrtc/rtc_base/flags.h but that's a lot of unnecessary work.

BUG=webrtc:7568
==========

to

==========
Make isac_fix_test use gtest (in a hacky way)

This test is the only remaining one that does not use gtest and that's
blocking some infra cleanup tasks. Ideally this test would use
webrtc/rtc_base/flags.h but that's a lot of unnecessary work.

This also replaces some exit() status codes - the logic behind this is if you
get incorrectly specified command line arguments, exit(1) is invoked for a
failure, because it's not a test failure, and if flag parsing was done properly,
it would not be a gtest failure anyway.

BUG=webrtc:7568
==========

[oprypin_webrtc, 2017-08-17 11:04:42]

Description was changed from

==========
Make isac_fix_test use gtest (in a hacky way)

This test is the only remaining one that does not use gtest and that's
blocking some infra cleanup tasks. Ideally this test would use
webrtc/rtc_base/flags.h but that's a lot of unnecessary work.

This also replaces some exit() status codes - the logic behind this is if you
get incorrectly specified command line arguments, exit(1) is invoked for a
failure, because it's not a test failure, and if flag parsing was done properly,
it would not be a gtest failure anyway.

BUG=webrtc:7568
==========

to

==========
Make isac_fix_test use gtest (in a hacky way)

This test is the only remaining one that does not use gtest and that's
blocking some infra cleanup tasks. Ideally this test would use
webrtc/rtc_base/flags.h but that's a lot of unnecessary work.

This also replaces some exit() status codes - the logic behind this is
if you get incorrectly specified command line arguments, exit(1) is
invoked for a failure, because it's not a test failure, and if flag
parsing was done properly, it would not be a gtest failure anyway.

BUG=webrtc:7568
==========

[oprypin_webrtc, 2017-08-17 11:05:09]

On 2017/08/17 11:02:20, oprypin_webrtc wrote:
> On 2017/08/17 09:53:29, kwiberg-webrtc wrote:
> > There are a whole bunch of exit() calls---don't you need to replace them too
> > with something gtesty?
> 
> Well it's not strictly necessary, the return status code would cause a failure
> anyway.
> I avoided doing this initially because this test needs a ton more work for a
> proper refactor.
> Done now.

Also added some info in commit message

[kwiberg-webrtc, 2017-08-17 11:16:14]

Thanks! lgtm

[hlundin-webrtc, 2017-08-17 11:18:12]

lgtm

[oprypin_webrtc, 2017-08-17 11:19:19]

The CQ bit was checked by oprypin@webrtc.org

[oprypin_webrtc, 2017-08-17 11:19:20]

The patchset sent to the CQ was uploaded after l-g-t-m from
kjellander@webrtc.org
Link to the patchset: https://codereview.webrtc.org/3000033002/#ps20001 (title:
"Replace  with gtest usage")

[commit-bot: I haz the power, 2017-08-17 11:19:25]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-08-17 12:03:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-08-17 12:03:01]

Try jobs failed on following builders:
  linux_baremetal on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_baremetal/builds/...)

[kjellander_webrtc, 2017-08-17 12:35:34]

On 2017/08/17 12:03:01, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   linux_baremetal on master.tryserver.webrtc (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.webrtc/builders/linux_baremetal/builds/...)

Meh, the buildbot recipe uses runtest.py for this test which in turn adds the
chromium specific flag --test-launcher-bot-mode to the test, which is an error.
The easiest workaround (see
https://cs.chromium.org/chromium/build/scripts/slave/runtest.py?rcl=d207ef4f6...)
aside from stop using runtest.py entirely (which would be nice), seems to be to
set annotate='graphing' instead of 'gtest' in the recipe. Edward can show you
this as he's working on this at the moment as well. Just make sure not to upload
the results to the perf dashboard (but I think the trybots cannot).

[oprypin_webrtc, 2017-08-17 12:38:00]

On 2017/08/17 12:35:34, kjellander_webrtc wrote:
> On 2017/08/17 12:03:01, commit-bot: I haz the power wrote:
> > Try jobs failed on following builders:
> >   linux_baremetal on master.tryserver.webrtc (JOB_FAILED,
> >
>
http://build.chromium.org/p/tryserver.webrtc/builders/linux_baremetal/builds/...)
> 
> Meh, the buildbot recipe uses runtest.py for this test which in turn adds the
> chromium specific flag --test-launcher-bot-mode to the test, which is an
error.
> The easiest workaround (see
>
https://cs.chromium.org/chromium/build/scripts/slave/runtest.py?rcl=d207ef4f6...)
> aside from stop using runtest.py entirely (which would be nice), seems to be
to
> set annotate='graphing' instead of 'gtest' in the recipe. Edward can show you
> this as he's working on this at the moment as well. Just make sure not to
upload
> the results to the perf dashboard (but I think the trybots cannot).

By the way, this test has been silently failing in all previous builds, I just
happened to change the exit code from 0 to 1 uncovering it.

[oprypin_webrtc, 2017-08-17 12:57:30]

On 2017/08/17 12:38:00, oprypin_webrtc wrote:
> On 2017/08/17 12:35:34, kjellander_webrtc wrote:
> > On 2017/08/17 12:03:01, commit-bot: I haz the power wrote:
> > > Try jobs failed on following builders:
> > >   linux_baremetal on master.tryserver.webrtc (JOB_FAILED,
> > >
> >
>
http://build.chromium.org/p/tryserver.webrtc/builders/linux_baremetal/builds/...)
> > 
> > Meh, the buildbot recipe uses runtest.py for this test which in turn adds
the
> > chromium specific flag --test-launcher-bot-mode to the test, which is an
> error.
> > The easiest workaround (see
> >
>
https://cs.chromium.org/chromium/build/scripts/slave/runtest.py?rcl=d207ef4f6...)
> > aside from stop using runtest.py entirely (which would be nice), seems to be
> to
> > set annotate='graphing' instead of 'gtest' in the recipe. Edward can show
you
> > this as he's working on this at the moment as well. Just make sure not to
> upload
> > the results to the perf dashboard (but I think the trybots cannot).
> 
> By the way, this test has been silently failing in all previous builds, I just
> happened to change the exit code from 0 to 1 uncovering it.

I have to partially undo this, and keep the error silenced as it was before. I
will fix this later.
I must unblock myself for the infra tasks, properly fixing this test has a
circular dependency on those very same infra tasks (stop sending unexpected
flags from the test infrastructure to our tests)

[oprypin_webrtc, 2017-08-17 12:58:25]

The CQ bit was checked by oprypin@webrtc.org

[oprypin_webrtc, 2017-08-17 12:58:25]

The patchset sent to the CQ was uploaded after l-g-t-m from
henrik.lundin@webrtc.org, kjellander@webrtc.org, kwiberg@webrtc.org
Link to the patchset: https://codereview.webrtc.org/3000033002/#ps40001 (title:
"Silence the error (as it was before)")

[commit-bot: I haz the power, 2017-08-17 12:58:40]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-08-17 15:25:29]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1502974705320880,
"parent_rev": "3958ed8e6f42ba436df86e42658b2156b3296575", "commit_rev":
"168576be1eac18c781de809ddedcd0b78e86daf3"}

[commit-bot: I haz the power, 2017-08-17 15:25:33]

Description was changed from

==========
Make isac_fix_test use gtest (in a hacky way)

This test is the only remaining one that does not use gtest and that's
blocking some infra cleanup tasks. Ideally this test would use
webrtc/rtc_base/flags.h but that's a lot of unnecessary work.

This also replaces some exit() status codes - the logic behind this is
if you get incorrectly specified command line arguments, exit(1) is
invoked for a failure, because it's not a test failure, and if flag
parsing was done properly, it would not be a gtest failure anyway.

BUG=webrtc:7568
==========

to

==========
Make isac_fix_test use gtest (in a hacky way)

This test is the only remaining one that does not use gtest and that's
blocking some infra cleanup tasks. Ideally this test would use
webrtc/rtc_base/flags.h but that's a lot of unnecessary work.

This also replaces some exit() status codes - the logic behind this is
if you get incorrectly specified command line arguments, exit(1) is
invoked for a failure, because it's not a test failure, and if flag
parsing was done properly, it would not be a gtest failure anyway.

BUG=webrtc:7568

Review-Url: https://codereview.webrtc.org/3000033002
Cr-Commit-Position: refs/heads/master@{#19388}
Committed:
https://chromium.googlesource.com/external/webrtc/+/168576be1eac18c781de809dd...
==========

[commit-bot: I haz the power, 2017-08-17 15:25:35]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/external/webrtc/+/168576be1eac18c781de809dd...