[ios] Removed web_shell_ prefix from EG tests.

[ios] Removed web_shell_ prefix from EG tests.

BUG=None

Committed: https://crrev.com/8f3e3352cc7ea89e0d7a6d92cb4be5943aa5ded9
Cr-Commit-Position: refs/heads/master@{#407249}

[Eugene But (OOO till 7-30), 2016-07-21 22:15:41]

eugenebut@chromium.org changed reviewers:
+ baxley@chromium.org

[baxley, 2016-07-22 15:16:57]

https://codereview.chromium.org/2173543002/diff/40001/ios/web/shell/test/page...
File ios/web/shell/test/page_state_egtest.mm (right):

https://codereview.chromium.org/2173543002/diff/40001/ios/web/shell/test/page...
ios/web/shell/test/page_state_egtest.mm:55: @interface PageStateTest :
ShellBaseTestCase
How does this fit in our general style?

I looked at other code inside ios/web/shell, and some of the files and classes
are prefixed with "shell". Do you think that makes sense here? Or do you think
it's better to just drop it all?

Sorry for not looking this up when you first asked about it.

[Eugene But (OOO till 7-30), 2016-07-22 16:09:33]

On 2016/07/22 15:16:57, baxley wrote:
>
https://codereview.chromium.org/2173543002/diff/40001/ios/web/shell/test/page...
> File ios/web/shell/test/page_state_egtest.mm (right):
> 
>
https://codereview.chromium.org/2173543002/diff/40001/ios/web/shell/test/page...
> ios/web/shell/test/page_state_egtest.mm:55: @interface PageStateTest :
> ShellBaseTestCase
> How does this fit in our general style?
> 
> I looked at other code inside ios/web/shell, and some of the files and classes
> are prefixed with "shell". Do you think that makes sense here? Or do you think
> it's better to just drop it all?
> 
> Sorry for not looking this up when you first asked about it.
If we prefix classes with Shell then we'll need to prefix files with shell_ as
well and that will make navigation in source code harder.
Classes prefixed with Shell have their counterpart with Chrome prefix so that
makes sense and we should keep those prefixes.
But I don't really think here is a value in prefixing tests.

[baxley, 2016-07-22 16:28:34]

On 2016/07/22 16:09:33, Eugene But wrote:
> On 2016/07/22 15:16:57, baxley wrote:
> >
>
https://codereview.chromium.org/2173543002/diff/40001/ios/web/shell/test/page...
> > File ios/web/shell/test/page_state_egtest.mm (right):
> > 
> >
>
https://codereview.chromium.org/2173543002/diff/40001/ios/web/shell/test/page...
> > ios/web/shell/test/page_state_egtest.mm:55: @interface PageStateTest :
> > ShellBaseTestCase
> > How does this fit in our general style?
> > 
> > I looked at other code inside ios/web/shell, and some of the files and
classes
> > are prefixed with "shell". Do you think that makes sense here? Or do you
think
> > it's better to just drop it all?
> > 
> > Sorry for not looking this up when you first asked about it.
> If we prefix classes with Shell then we'll need to prefix files with shell_ as
> well and that will make navigation in source code harder.
Why would it make it harder if the name was prefixed with "shell_" when the
source that it is near also has that prefix?

> Classes prefixed with Shell have their counterpart with Chrome prefix so that
> makes sense and we should keep those prefixes.
> But I don't really think here is a value in prefixing tests.
I don't quite understand the distinction between tests and non-tests, and why
there is value for one and not the other. I don't think saying Chrome does it
for non-tests, but doesn't for tests is a good reason on its own. We're setting
the style for tests, so we should do what makes the most sense.

So I guess my question is... Why should code all have a prefix, but then tests
that live next to the code not have the prefix (without using Chrome as an
example, since we're at the start with Earl Grey tests where we should set the
precedence and not follow what's there)?

[Eugene But (OOO till 7-30), 2016-07-22 16:40:04]

>>> 
Why would it make it harder if the name was prefixed with "shell_" when the
source that it is near also has that prefix?
It takes time to read the prefix and if every single file in the directory has
this prefix then it will take more time to scan through files.

>>> I don't quite understand the distinction between tests and non-tests, and
why there is value for one and not the other.
There is a base class called WebClient and it has 2 concrete implementations:
ChromeWebClient and ShellWebClient.
You need a prefix to distinguish between WebClient and ShellWebClient.
There is no NavigationTest base class with concrete ShellNavigationTest and
ChromeNavigationTest subclasses, so enforcing prefix is not necessary.

I would see a value in case if we had large number of chrome and shell tests
with the same name. Do you think it's going to be the case?

[baxley, 2016-07-22 18:48:54]

lgtm

[Eugene But (OOO till 7-30), 2016-07-22 19:01:21]

Thanks!

[Eugene But (OOO till 7-30), 2016-07-22 19:01:24]

The CQ bit was checked by eugenebut@chromium.org

[commit-bot: I haz the power, 2016-07-22 19:02:38]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-22 20:29:26]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-07-22 20:32:16]

Description was changed from

==========
[ios] Removed web_shell_ prefix from EG tests.

BUG=None
==========

to

==========
[ios] Removed web_shell_ prefix from EG tests.

BUG=None

Committed: https://crrev.com/8f3e3352cc7ea89e0d7a6d92cb4be5943aa5ded9
Cr-Commit-Position: refs/heads/master@{#407249}
==========

[commit-bot: I haz the power, 2016-07-22 20:32:17]

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