Rewrite text field context menu tests using accessiblity framework....

Some rewrited text field context menu tests using accessiblity framework.

BUG=none
TEST=none

[kkania, 2010-11-01 19:13:10]

just a couple more comments

http://codereview.chromium.org/3656002/diff/1/5
File chrome_frame/test/chrome_frame_test_utils.h (right):

http://codereview.chromium.org/3656002/diff/1/5#newcode154
chrome_frame/test/chrome_frame_test_utils.h:154: // Set text data to clipboard.
// Puts the given text data on the clipboard. All previous items on the
clipboard are removed.

http://codereview.chromium.org/3656002/diff/1/6
File chrome_frame/test/ui_test.cc (right):

http://codereview.chromium.org/3656002/diff/1/6#newcode354
chrome_frame/test/ui_test.cc:354: // Clear clipboard to make sure ther is no
effect from previous tests.
misspell: ther should be there

http://codereview.chromium.org/3656002/diff/1/6#newcode807
chrome_frame/test/ui_test.cc:807:
.WillOnce(AccLeftClick(AccObjectMatcher(L"Redo*")));
This could be much clearer if you put this last WillOnce in a separate
EXPECT_CALL after the next EXPECT_CALL. You will have to use InSequence.

http://codereview.chromium.org/3656002/diff/1/6#newcode815
chrome_frame/test/ui_test.cc:815: // After redo operation is done, verify that
text field value is set to 'A'
verify that the text field value is reset to its original value

[mdu, 2010-11-05 23:13:29]

updated due to ken's comments, merged with the latest source code.

[kkania, 2010-11-09 22:45:21]

LGTM

http://codereview.chromium.org/3656002/diff/14001/chrome_frame/test/data/cont...
File chrome_frame/test/data/context_menu.html (right):

http://codereview.chromium.org/3656002/diff/14001/chrome_frame/test/data/cont...
chrome_frame/test/data/context_menu.html:35: function
verifyTextFieldContents(event) {
Now that we only have one action, let's simplify this. Can you get rid of the
first two functions, and just move the necessary stuff into here?

http://codereview.chromium.org/3656002/diff/14001/chrome_frame/test/ui_test.cc
File chrome_frame/test/ui_test.cc (right):

http://codereview.chromium.org/3656002/diff/14001/chrome_frame/test/ui_test.c...
chrome_frame/test/ui_test.cc:406: std::wstring kContextMenuPageUrl;
This should be changed to context_menu_page_url

[mdu, 2010-11-09 23:32:06]

Hi Amit, 

Context menu tests have been rewritten with accessibility framework and Ken has
reviewed them, could you spend some time to give them a quick review?

Thanks

[amit, 2010-11-09 23:45:27]

Looks awesome, thanks! LGTM with a nit.

http://codereview.chromium.org/3656002/diff/22001/chrome_frame/test/ui_test.cc
File chrome_frame/test/ui_test.cc (right):

http://codereview.chromium.org/3656002/diff/22001/chrome_frame/test/ui_test.c...
chrome_frame/test/ui_test.cc:700:
server_mock_.ExpectAndServeAnyRequests(CFInvocation::MetaTag());
There's a lot of common code between the following few tests, is it possible to
easily factor it out?

[mdu, 2010-11-10 01:21:16]

http://codereview.chromium.org/3656002/diff/22001/chrome_frame/test/ui_test.cc
File chrome_frame/test/ui_test.cc (right):

http://codereview.chromium.org/3656002/diff/22001/chrome_frame/test/ui_test.c...
chrome_frame/test/ui_test.cc:700:
server_mock_.ExpectAndServeAnyRequests(CFInvocation::MetaTag());
On 2010/11/09 23:45:27, amit wrote:
> There's a lot of common code between the following few tests, is it possible
to
> easily factor it out?

To do this we have to split ContextMenuTest into two separate fixtures because
currently there are some IE tests in it, this will affect the existing tests
written by others. Is it ok not to factor it out?

[amit, 2010-11-10 19:00:20]

On Tue, Nov 9, 2010 at 5:21 PM, <mdu@chromium.org> wrote:

>
>
> http://codereview.chromium.org/3656002/diff/22001/chrome_frame/test/ui_test.cc
> File chrome_frame/test/ui_test.cc (right):
>
>
>
http://codereview.chromium.org/3656002/diff/22001/chrome_frame/test/ui_test.c...
> chrome_frame/test/ui_test.cc:700:
> server_mock_.ExpectAndServeAnyRequests(CFInvocation::MetaTag());
> On 2010/11/09 23:45:27, amit wrote:
>
>> There's a lot of common code between the following few tests, is it
>>
> possible to
>
>> easily factor it out?
>>
>
> To do this we have to split ContextMenuTest into two separate fixtures
> because currently there are some IE tests in it, this will affect the
> existing tests written by others. Is it ok not to factor it out?

OK for now but think future about changes to the mock classes that will
result in identical modifications across those tests.

>
>
> http://codereview.chromium.org/3656002/
>

[amit, 2010-11-10 19:06:18]

On Tue, Nov 9, 2010 at 5:21 PM, <mdu@chromium.org> wrote:

>
>
> http://codereview.chromium.org/3656002/diff/22001/chrome_frame/test/ui_test.cc
> File chrome_frame/test/ui_test.cc (right):
>
>
>
http://codereview.chromium.org/3656002/diff/22001/chrome_frame/test/ui_test.c...
> chrome_frame/test/ui_test.cc:700:
> server_mock_.ExpectAndServeAnyRequests(CFInvocation::MetaTag());
> On 2010/11/09 23:45:27, amit wrote:
>
>> There's a lot of common code between the following few tests, is it
>>
> possible to
>
>> easily factor it out?
>>
>
> To do this we have to split ContextMenuTest into two separate fixtures
> because currently there are some IE tests in it, this will affect the
> existing tests written by others. Is it ok not to factor it out?

OK for now but think future about changes to the mock classes that will
result in identical modifications across those tests.

>
>
> http://codereview.chromium.org/3656002/
>

[amit, 2010-11-10 19:16:21]

On Tue, Nov 9, 2010 at 5:21 PM, <mdu@chromium.org> wrote:

>
>
> http://codereview.chromium.org/3656002/diff/22001/chrome_frame/test/ui_test.cc
> File chrome_frame/test/ui_test.cc (right):
>
>
>
http://codereview.chromium.org/3656002/diff/22001/chrome_frame/test/ui_test.c...
> chrome_frame/test/ui_test.cc:700:
> server_mock_.ExpectAndServeAnyRequests(CFInvocation::MetaTag());
> On 2010/11/09 23:45:27, amit wrote:
>
>> There's a lot of common code between the following few tests, is it
>>
> possible to
>
>> easily factor it out?
>>
>
> To do this we have to split ContextMenuTest into two separate fixtures
> because currently there are some IE tests in it, this will affect the
> existing tests written by others. Is it ok not to factor it out?

OK for now but think future about changes to the mock classes that will
result in identical modifications across those tests.

>
>
> http://codereview.chromium.org/3656002/
>

[amit, 2010-11-10 19:24:18]

On Tue, Nov 9, 2010 at 5:21 PM, <mdu@chromium.org> wrote:

>
>
> http://codereview.chromium.org/3656002/diff/22001/chrome_frame/test/ui_test.cc
> File chrome_frame/test/ui_test.cc (right):
>
>
>
http://codereview.chromium.org/3656002/diff/22001/chrome_frame/test/ui_test.c...
> chrome_frame/test/ui_test.cc:700:
> server_mock_.ExpectAndServeAnyRequests(CFInvocation::MetaTag());
> On 2010/11/09 23:45:27, amit wrote:
>
>> There's a lot of common code between the following few tests, is it
>>
> possible to
>
>> easily factor it out?
>>
>
> To do this we have to split ContextMenuTest into two separate fixtures
> because currently there are some IE tests in it, this will affect the
> existing tests written by others. Is it ok not to factor it out?

OK for now but think future about changes to the mock classes that will
result in identical modifications across those tests.

>
>
> http://codereview.chromium.org/3656002/
>

[amit, 2010-11-10 19:26:23]

On Tue, Nov 9, 2010 at 5:21 PM, <mdu@chromium.org> wrote:

>
>
> http://codereview.chromium.org/3656002/diff/22001/chrome_frame/test/ui_test.cc
> File chrome_frame/test/ui_test.cc (right):
>
>
>
http://codereview.chromium.org/3656002/diff/22001/chrome_frame/test/ui_test.c...
> chrome_frame/test/ui_test.cc:700:
> server_mock_.ExpectAndServeAnyRequests(CFInvocation::MetaTag());
> On 2010/11/09 23:45:27, amit wrote:
>
>> There's a lot of common code between the following few tests, is it
>>
> possible to
>
>> easily factor it out?
>>
>
> To do this we have to split ContextMenuTest into two separate fixtures
> because currently there are some IE tests in it, this will affect the
> existing tests written by others. Is it ok not to factor it out?

OK for now but think future about changes to the mock classes that will
result in identical modifications across those tests.

>
>
> http://codereview.chromium.org/3656002/
>

[amit, 2010-11-10 19:36:26]

On Tue, Nov 9, 2010 at 5:21 PM, <mdu@chromium.org> wrote:

>
>
> http://codereview.chromium.org/3656002/diff/22001/chrome_frame/test/ui_test.cc
> File chrome_frame/test/ui_test.cc (right):
>
>
>
http://codereview.chromium.org/3656002/diff/22001/chrome_frame/test/ui_test.c...
> chrome_frame/test/ui_test.cc:700:
> server_mock_.ExpectAndServeAnyRequests(CFInvocation::MetaTag());
> On 2010/11/09 23:45:27, amit wrote:
>
>> There's a lot of common code between the following few tests, is it
>>
> possible to
>
>> easily factor it out?
>>
>
> To do this we have to split ContextMenuTest into two separate fixtures
> because currently there are some IE tests in it, this will affect the
> existing tests written by others. Is it ok not to factor it out?

OK for now but think future about changes to the mock classes that will
result in identical modifications across those tests.

>
>
> http://codereview.chromium.org/3656002/
>

[amit, 2010-11-10 19:46:28]

On Tue, Nov 9, 2010 at 5:21 PM, <mdu@chromium.org> wrote:

>
>
> http://codereview.chromium.org/3656002/diff/22001/chrome_frame/test/ui_test.cc
> File chrome_frame/test/ui_test.cc (right):
>
>
>
http://codereview.chromium.org/3656002/diff/22001/chrome_frame/test/ui_test.c...
> chrome_frame/test/ui_test.cc:700:
> server_mock_.ExpectAndServeAnyRequests(CFInvocation::MetaTag());
> On 2010/11/09 23:45:27, amit wrote:
>
>> There's a lot of common code between the following few tests, is it
>>
> possible to
>
>> easily factor it out?
>>
>
> To do this we have to split ContextMenuTest into two separate fixtures
> because currently there are some IE tests in it, this will affect the
> existing tests written by others. Is it ok not to factor it out?

OK for now but think future about changes to the mock classes that will
result in identical modifications across those tests.

>
>
> http://codereview.chromium.org/3656002/
>

[amit, 2010-11-10 20:01:07]

On Tue, Nov 9, 2010 at 5:21 PM, <mdu@chromium.org> wrote:

>
>
> http://codereview.chromium.org/3656002/diff/22001/chrome_frame/test/ui_test.cc
> File chrome_frame/test/ui_test.cc (right):
>
>
>
http://codereview.chromium.org/3656002/diff/22001/chrome_frame/test/ui_test.c...
> chrome_frame/test/ui_test.cc:700:
> server_mock_.ExpectAndServeAnyRequests(CFInvocation::MetaTag());
> On 2010/11/09 23:45:27, amit wrote:
>
>> There's a lot of common code between the following few tests, is it
>>
> possible to
>
>> easily factor it out?
>>
>
> To do this we have to split ContextMenuTest into two separate fixtures
> because currently there are some IE tests in it, this will affect the
> existing tests written by others. Is it ok not to factor it out?

OK for now but think future about changes to the mock classes that will
result in identical modifications across those tests.

>
>
> http://codereview.chromium.org/3656002/
>