Remove hardcoded tests URLs from apptests.

Remove hardcoded tests URLs from apptests.

Hardcoded URLs cause problems if the test is given a different name. This occurs when
compiling the test with the NaCl toolchain.

BUG=401761
R=jamesr@chromium.org

Committed: https://chromium.googlesource.com/external/mojo/+/24d8e69e836e770114a1ed16e1f7395d10c3ce5e

[Nick Bray (chromium), 2015-03-10 00:24:27]

ncbray@chromium.org changed reviewers:
+ jamesr@chromium.org

[jamesr, 2015-03-10 00:28:27]

Hmm, can't you resolve this by specifying URL mappings when running the nacl
tests?

[Nick Bray (chromium), 2015-03-10 00:39:56]

On 2015/03/10 00:28:27, jamesr wrote:
> Hmm, can't you resolve this by specifying URL mappings when running the nacl
> tests?

Theoretically possible, but 1) it becomes harder to run the test because it
_must_ have a flag and 2) conceptually these tests are just "connecting to
themselves" so hardcoding the URL is simply lazy rather than intended.

[jamesr, 2015-03-10 00:46:08]

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

[jamesr, 2015-03-10 00:46:09]

"mojob.py test" takes care of setting up the command line, so that's not an
issue.  Apptests in general require a fair number of flags to run and the
framework prints out the command line used for a test whenever it fails so it's
easy to run manually.

I'm not sure that this is purely a case of laziness.  Scott - what are these
view manager/window manager tests intending by using these URLs?

[sky, 2015-03-10 01:55:57]

On 2015/03/10 00:46:09, jamesr wrote:
> "mojob.py test" takes care of setting up the command line, so that's not an
> issue.  Apptests in general require a fair number of flags to run and the
> framework prints out the command line used for a test whenever it fails so
it's
> easy to run manually.
> 
> I'm not sure that this is purely a case of laziness.  Scott - what are these
> view manager/window manager tests intending by using these URLs?

They are connecting back to themself.

[qsr, 2015-03-10 10:24:54]

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

[qsr, 2015-03-10 10:24:55]

https://codereview.chromium.org/996533002/diff/1/mojo/public/cpp/application/...
File mojo/public/cpp/application/application_test_base.h (right):

https://codereview.chromium.org/996533002/diff/1/mojo/public/cpp/application/...
mojo/public/cpp/application/application_test_base.h:25: const String& Url();
I don't think you need this method. As you extend ApplicationTestBase, you can
get this with:

application_impl()->url()

[viettrungluu, 2015-03-10 15:31:51]

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

[viettrungluu, 2015-03-10 15:31:52]

https://codereview.chromium.org/996533002/diff/1/mojo/public/cpp/application/...
File mojo/public/cpp/application/lib/application_test_base.cc (right):

https://codereview.chromium.org/996533002/diff/1/mojo/public/cpp/application/...
mojo/public/cpp/application/lib/application_test_base.cc:20: String g_url;
No comment?

(Also, the seemingly-random order of declaration of the globals and
initialization/copying in Initialize() below -- not corresponding to (e.g.)
argument order or anything) -- triggers my OCD. But since that's a existing
condition, feel free to ignore me.)

https://codereview.chromium.org/996533002/diff/1/mojo/public/cpp/application/...
mojo/public/cpp/application/lib/application_test_base.cc:150:
application_impl_->Initialize(g_shell.Pass(), g_args.Clone(), g_url);
I agree that this is probably right, and that passing the empty string was a
bug.

[Nick Bray (chromium), 2015-03-10 19:39:00]

PTAL

https://codereview.chromium.org/996533002/diff/1/mojo/public/cpp/application/...
File mojo/public/cpp/application/application_test_base.h (right):

https://codereview.chromium.org/996533002/diff/1/mojo/public/cpp/application/...
mojo/public/cpp/application/application_test_base.h:25: const String& Url();
On 2015/03/10 10:24:55, qsr wrote:
> I don't think you need this method. As you extend ApplicationTestBase, you can
> get this with:
> 
> application_impl()->url()

Done.

https://codereview.chromium.org/996533002/diff/1/mojo/public/cpp/application/...
File mojo/public/cpp/application/lib/application_test_base.cc (right):

https://codereview.chromium.org/996533002/diff/1/mojo/public/cpp/application/...
mojo/public/cpp/application/lib/application_test_base.cc:20: String g_url;
On 2015/03/10 15:31:52, viettrungluu wrote:
> No comment?
> 
> (Also, the seemingly-random order of declaration of the globals and
> initialization/copying in Initialize() below -- not corresponding to (e.g.)
> argument order or anything) -- triggers my OCD. But since that's a existing
> condition, feel free to ignore me.)

Done.

[jamesr, 2015-03-10 19:44:30]

lgtm

[Nick Bray (chromium), 2015-03-10 19:53:11]

Committed patchset #2 (id:20001) manually as
24d8e69e836e770114a1ed16e1f7395d10c3ce5e (presubmit successful).