Plumb commandline arguments for mojo_example_apptests.

Plumb commandline arguments for mojo_example_apptests.

Use the ApplicationImpl::args() for InitGoogleTest.
(plumbs GTEST args from the mojo_shell commandline)

BUG=392646
TEST=GTEST args work like this> mojo_shell "mojo:mojo_example_apptests --gtest_filter=ExampleApptest.RunCallbackViaService --gtest_repeat=2"
R=hansmuller@chromium.org,davemoore@chromium.org

Committed: https://crrev.com/ef5eefe35befb1bd202fead20f282f91e645387d
Cr-Commit-Position: refs/heads/master@{#299001}

[msw, 2014-10-06 19:50:59]

Patchset #2 (id:20001) has been deleted

[msw, 2014-10-06 19:57:51]

msw@chromium.org changed reviewers:
+ hansmuller@chromium.org

[msw, 2014-10-06 20:00:14]

Hey Hans, can you take a look? I haven't found a utility method to support
generating argv from an std::vector<std::string>, maybe i'll generalize this one
for arbitrary mojo_apptests (in a follow up CL)?

[hansmuller, 2014-10-06 21:08:52]

I agree that creating a a C friendly mutable argv from the ApplicationImpl's
Array of Strings would be a useful little utility to share from somewhere.

https://codereview.chromium.org/626033003/diff/40001/mojo/examples/apptest/ex...
File mojo/examples/apptest/example_apptest.cc (right):

https://codereview.chromium.org/626033003/diff/40001/mojo/examples/apptest/ex...
mojo/examples/apptest/example_apptest.cc:97: for (int i = 0; i < argc; ++i)
You might want to use size_t instead of int for the loop index etc.

I don't think you need to convert the Mojo args array to a std::vector, you
should be able to copy app.args() into the GTest (mutable) argv roughly like
this:

  for (size_t i = 0; i < argc; ++)
    argv[i] = app.args()[i].data();

[msw, 2014-10-06 21:31:40]

msw@chromium.org changed reviewers:
+ sky@chromium.org

[msw, 2014-10-06 21:32:18]

msw@chromium.org changed reviewers:
+ davemoore@chromium.org
- sky@chromium.org

[msw, 2014-10-06 21:32:33]

Dave, can you please take a look? Thanks!

https://codereview.chromium.org/626033003/diff/40001/mojo/examples/apptest/ex...
File mojo/examples/apptest/example_apptest.cc (right):

https://codereview.chromium.org/626033003/diff/40001/mojo/examples/apptest/ex...
mojo/examples/apptest/example_apptest.cc:97: for (int i = 0; i < argc; ++i)
On 2014/10/06 21:08:52, hansmuller wrote:
> You might want to use size_t instead of int for the loop index etc.

InitGoogleTest eventually requires an int* :-/

> I don't think you need to convert the Mojo args array to a std::vector, you
> should be able to copy app.args() into the GTest (mutable) argv roughly like
> this:
> 
>   for (size_t i = 0; i < argc; ++)
>     argv[i] = app.args()[i].data();

I don't think that'll work, because InitGoogleTest has the baffling behavior of
removing GTEST arguments from |argv| and decrementing |argc|, so this can't pass
the App's const string data directly. lmk if I'm missing something.

[viettrungluu, 2014-10-06 22:31:58]

On 2014/10/06 21:32:33, msw wrote:
> Dave, can you please take a look? Thanks!
> 
>
https://codereview.chromium.org/626033003/diff/40001/mojo/examples/apptest/ex...
> File mojo/examples/apptest/example_apptest.cc (right):
> 
>
https://codereview.chromium.org/626033003/diff/40001/mojo/examples/apptest/ex...
> mojo/examples/apptest/example_apptest.cc:97: for (int i = 0; i < argc; ++i)
> On 2014/10/06 21:08:52, hansmuller wrote:
> > You might want to use size_t instead of int for the loop index etc.
> 
> InitGoogleTest eventually requires an int* :-/
> 
> > I don't think you need to convert the Mojo args array to a std::vector, you
> > should be able to copy app.args() into the GTest (mutable) argv roughly like
> > this:
> > 
> >   for (size_t i = 0; i < argc; ++)
> >     argv[i] = app.args()[i].data();
> 
> I don't think that'll work, because InitGoogleTest has the baffling behavior
of
> removing GTEST arguments from |argv| and decrementing |argc|, so this can't
pass
> the App's const string data directly. lmk if I'm missing something.

InitGoogleTest() isn't const-correct, but I believe it only modifies the
pointers of argv, not the string data of argv. (I.e., it could take a const
char** argument, but not a const char* const* argument.)

[msw, 2014-10-06 22:50:49]

On 2014/10/06 22:31:58, viettrungluu wrote:
> On 2014/10/06 21:32:33, msw wrote:
> > Dave, can you please take a look? Thanks!
> > 
> >
>
https://codereview.chromium.org/626033003/diff/40001/mojo/examples/apptest/ex...
> > File mojo/examples/apptest/example_apptest.cc (right):
> > 
> >
>
https://codereview.chromium.org/626033003/diff/40001/mojo/examples/apptest/ex...
> > mojo/examples/apptest/example_apptest.cc:97: for (int i = 0; i < argc; ++i)
> > On 2014/10/06 21:08:52, hansmuller wrote:
> > > You might want to use size_t instead of int for the loop index etc.
> > 
> > InitGoogleTest eventually requires an int* :-/
> > 
> > > I don't think you need to convert the Mojo args array to a std::vector,
you
> > > should be able to copy app.args() into the GTest (mutable) argv roughly
like
> > > this:
> > > 
> > >   for (size_t i = 0; i < argc; ++)
> > >     argv[i] = app.args()[i].data();
> > 
> > I don't think that'll work, because InitGoogleTest has the baffling behavior
> of
> > removing GTEST arguments from |argv| and decrementing |argc|, so this can't
> pass
> > the App's const string data directly. lmk if I'm missing something.
> 
> InitGoogleTest() isn't const-correct, but I believe it only modifies the
> pointers of argv, not the string data of argv. (I.e., it could take a const
> char** argument, but not a const char* const* argument.)

Fair enough; I updated the CL to use const_cast'ed ApplicationImpl::args data
directly.

[msw, 2014-10-07 23:16:21]

Dave, please take a look as you have time; thanks!

[DaveMoore, 2014-10-08 19:03:09]

lgtm

[msw, 2014-10-08 19:04:24]

The CQ bit was checked by msw@chromium.org

[commit-bot: I haz the power, 2014-10-08 19:05:32]

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

[commit-bot: I haz the power, 2014-10-08 19:19:00]

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

[commit-bot: I haz the power, 2014-10-08 19:19:01]

Try jobs failed on following builders:
  android_chromium_gn_compile_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)

[msw, 2014-10-08 19:28:29]

The CQ bit was checked by msw@chromium.org

[commit-bot: I haz the power, 2014-10-08 19:30:19]

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

[commit-bot: I haz the power, 2014-10-08 20:00:41]

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

[commit-bot: I haz the power, 2014-10-08 20:00:42]

Try jobs failed on following builders:
  win_chromium_x64_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[msw, 2014-10-08 20:03:18]

The CQ bit was checked by msw@chromium.org

[commit-bot: I haz the power, 2014-10-08 20:06:44]

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

[commit-bot: I haz the power, 2014-10-09 03:43:48]

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

[commit-bot: I haz the power, 2014-10-09 03:43:49]

Try jobs failed on following builders:
  win_chromium_x64_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[msw, 2014-10-09 05:24:30]

The CQ bit was checked by msw@chromium.org

[msw, 2014-10-09 05:24:48]

The CQ bit was unchecked by msw@chromium.org

[msw, 2014-10-09 18:46:33]

The CQ bit was checked by msw@chromium.org

[commit-bot: I haz the power, 2014-10-09 18:47:13]

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

[commit-bot: I haz the power, 2014-10-09 19:01:46]

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

[commit-bot: I haz the power, 2014-10-09 19:01:48]

Try jobs failed on following builders:
  linux_chromium_compile_dbg_32 on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[msw, 2014-10-09 21:31:18]

The CQ bit was checked by msw@chromium.org

[commit-bot: I haz the power, 2014-10-09 21:31:38]

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

[commit-bot: I haz the power, 2014-10-09 22:49:29]

Committed patchset #7 (id:300001)

[commit-bot: I haz the power, 2014-10-09 22:52:22]

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