Don't enable app shims in tests. (Mac)

Don't enable app shims in tests. (Mac)

This causes problems for try bots as the app shims cannot connect to
the test chrome processes. Also are created in user-local (rather than
test-local) locations.

BUG=334079
TEST=Tests that install apps (e.g.PlatformAppBrowserTest) should
not create app shims (in ~/Applications/Chromium Apps/).

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=245076

[mlamouri (slow - plz ping), 2014-01-15 02:56:12]

Hmm, shouldn't we keep the apps shim if we can for interactive_ui_tests?
Otherwise, testing that a shortcut works will not be possible on MacOS and that
would be bad :(

[jackhou1, 2014-01-15 03:42:24]

On 2014/01/15 02:56:12, Mounir Lamouri wrote:
> Hmm, shouldn't we keep the apps shim if we can for interactive_ui_tests?
> Otherwise, testing that a shortcut works will not be possible on MacOS and
that
> would be bad :(

At the moment we test the IPC channel and Chrome side behavior, but not the shim
itself. I think for an interactive test of the shim, it would be better to
introduce a way to enable shims for just that test. It's a common pattern at the
moment to install apps in tests without uninstalling them. With shims enabled,
this would create shims on the local machine which are never removed.

[tapted, 2014-01-15 04:26:13]

lgtm for keeping the bots happy (assuming the CQ is also happy)

I think a full integration test for the "shortcut" on Mac would need a different
test harness completely, since it won't be looking for Chrome in the right place
anyway, in order to find the test process. So, right now, it's just junk.

https://codereview.chromium.org/135853008/diff/1/apps/app_shim/app_shim_mac.cc
File apps/app_shim/app_shim_mac.cc (right):

https://codereview.chromium.org/135853008/diff/1/apps/app_shim/app_shim_mac.c...
apps/app_shim/app_shim_mac.cc:13: return
!(CommandLine::ForCurrentProcess()->HasSwitch(switches::kTestType) ||
nit: should have a comment: ~avoid installing app shims to ~/Applications in
tests, since they will not be cleaned up.

[mlamouri (slow - plz ping), 2014-01-15 04:58:43]

On 2014/01/15 04:26:13, tapted wrote:
> lgtm for keeping the bots happy (assuming the CQ is also happy)
> 
> I think a full integration test for the "shortcut" on Mac would need a
different
> test harness completely, since it won't be looking for Chrome in the right
place
> anyway, in order to find the test process. So, right now, it's just junk.

Do you mean that if I open an app in an interactive_ui_tests and simulate some
key events it will not trigger the expect action on MacOS with the app shims
enabled?

[tapted, 2014-01-15 05:03:52]

On 2014/01/15 04:58:43, Mounir Lamouri wrote:
> On 2014/01/15 04:26:13, tapted wrote:
> > lgtm for keeping the bots happy (assuming the CQ is also happy)
> > 
> > I think a full integration test for the "shortcut" on Mac would need a
> different
> > test harness completely, since it won't be looking for Chrome in the right
> place
> > anyway, in order to find the test process. So, right now, it's just junk.
> 
> Do you mean that if I open an app in an interactive_ui_tests and simulate some
> key events it will not trigger the expect action on MacOS with the app shims
> enabled?

The shim process just provides Dock/Spotlight/Finder integration - it has no
windows of its own, and never actually gets to keep keyboard focus.

[jackhou1, 2014-01-15 05:06:31]

https://codereview.chromium.org/135853008/diff/1/apps/app_shim/app_shim_mac.cc
File apps/app_shim/app_shim_mac.cc (right):

https://codereview.chromium.org/135853008/diff/1/apps/app_shim/app_shim_mac.c...
apps/app_shim/app_shim_mac.cc:13: return
!(CommandLine::ForCurrentProcess()->HasSwitch(switches::kTestType) ||
On 2014/01/15 04:26:13, tapted wrote:
> nit: should have a comment: ~avoid installing app shims to ~/Applications in
> tests, since they will not be cleaned up.

Done.

[commit-bot: I haz the power, 2014-01-15 08:27:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jackhou@chromium.org/135853008/100001

[commit-bot: I haz the power, 2014-01-16 02:48:44]

Change committed as 245076