Re: Status of review for PR-324

Sorry for the delay.  I've now finished reviewing the tests 1-5 (including
running them on IE11 and debugging some of the failures I was getting).
 I've added a number of comments to the PR.

My biggest high-level comments (which appear to impact all the tests in the
submission) are:

1) Most of the tests appear to cause the harness to timeout because they
call "setup({ explicit_done: true })" but never call the global done()
handler.  For the most part we should be able to remove this line to
prevent the timeout - just need to ensure that at least one async test is
started before load, and all the testing has completed by the time the last
async test has it's done method called.

2) I think the test files should have coarser granularity.  In many cases
an entire file is used (generally duplicated from another test - itself a
sign there is a problem) just to check one simple API or minor variation on
a scenario tested in another file.  Since these are manual tests, each test
file imposes a significant burden on running the suite.  Ideally there'd be
one file for each action we want the user to perform and then we run all
tests for that action.  If we want to break it up a little further (eg.
into a few logical groups like "capture tests") then that's OK with me too.
 But if it takes more than a minute for an operator to walk through all the
scenarios and run all the pointer event tests then I think we're doing
something wrong (and the tests will be unlikely to get run in practice).
 For the capture tests I reviewed, it looks like it would be simple to
combine all the scenarios into the one capture.html file.  Thoughts?

Rick


On Tue, Oct 22, 2013 at 8:44 AM, Arthur Barstow <art.barstow@nokia.com>wrote:

> Thanks Cathy!
>
> I think this means the following files in [PR324] have _not_ yet been
> reviewed (http mirror is [PR324-Mirror]):
>
> 1. capture.html
> 2. pointerevent_**gotpointercapture_before_**first_pointerevent.html
> 3. pointerevent_**releasePointerCapture_events_**to_original_target.html
> 4. pointerevent_**releasePointerCapture_invalid_**pointerId.html
> 5. pointerevent_**setPointerCapture_invalid_**pointerId.html
>
> 6. pointerevent_pointerLeave_**mouse.html
> 7. pointerevent_pointerLeave_pen.**html
> 8. pointerevent_pointerLeave_**touch.html
>
> Rick, Scott, Olli, Sangwhan, Others - can you please commit to reviewing
> some set of these files?
>
> Jacob, Asir - please see the comments from Cathy and Matt and reply and/or
> submit an update, accordingly.
>
> -Thanks, ArtB
>
> [PR324] <https://github.com/w3c/web-**platform-tests/pull/324<https://github.com/w3c/web-platform-tests/pull/324>
> >
> [PR324-Mirror] <http://w3c-test.org/web-**platform-tests/submissions/**
> 324/pointerevents/<http://w3c-test.org/web-platform-tests/submissions/324/pointerevents/>
> >
>
>
> On 10/21/13 4:02 PM, Chan Cathy (Nokia-CIC/Boston) wrote:
>
>> Hi all,
>>
>> I've reviewed a bunch of test files and provided my comments at the PR
>> link. I've also provided some general comments.
>>
>> The following tests were reviewed and IMO require some changes (on top of
>> those from the general comments).
>> pointercancel.html
>> pointerevent_pointerEnter_**does_not_bubble.html
>> pointerevent_pointerEnter_**noHover.html
>> pointerevent_pointerLeave_**does_not_bubble.html
>> pointerevent_pointerMove_**isPrimary_same_as_pointerDown.**html
>> pointerevent_support.js
>> pointerevent_pointerType_**mouse.html
>> pointerevent_pointerType_pen.**html
>> pointerevent_pointerType_**touch.html
>> pointerout.html
>> pointerup.html
>>
>> The following tests were reviewed as ok, but might require changes as per
>> the general comments.
>> pointerevent_pointerEnter.html
>> pointerevent_pointerOver.html
>>
>> Regards, Cathy.
>>
>>
>>  -----Original Message-----
>>> From: Barstow Art (Nokia-CIC/Boston)
>>> Sent: Monday, October 07, 2013 12:13 PM
>>> To: public-pointer-events@w3.org
>>> Subject: Status of review for PR-324
>>>
>>> Cathy and Matt reviewed the following tests Jacob included in [PR-324]
>>> (thanks Cathy and Matt!):
>>>
>>> Cathy:
>>> pointercancel.html
>>> pointerevent_pointerEnter.html
>>> pointerevent_pointerEnter_**does_not_bubble.html
>>> pointerevent_pointerEnter_**noHover.html
>>> pointerevent_pointerLeave_**does_not_bubble.html
>>>
>>> Matt:
>>> pointerevent_pointerMove_**isPrimary_same_as_pointerDown.**html
>>> pointerevent_pointerOver.html
>>> pointerevent_support.js
>>> pointerout.html
>>> pointerup.html
>>>
>>> If anyone else can review the other tests, please do so and include your
>>> comments to PR-324 (even if just a "+1/OK") :
>>>
>>> capture.html
>>> pointerevent_**gotpointercapture_before_**first_pointerevent.html
>>> pointerevent_**releasePointerCapture_events_**to_original_target.html
>>> pointerevent_**releasePointerCapture_invalid_**pointerId.html
>>> pointerevent_**setPointerCapture_invalid_**pointerId.html
>>>
>>> pointerevent_pointerLeave_**mouse.html
>>> pointerevent_pointerLeave_pen.**html
>>> pointerevent_pointerLeave_**touch.html
>>> pointerevent_pointerType_**mouse.html
>>> pointerevent_pointerType_pen.**html
>>> pointerevent_pointerType_**touch.html
>>>
>>> -Thanks, AB
>>>
>>> [PR-324] <https://github.com/w3c/web-**platform-tests/pull/324<https://github.com/w3c/web-platform-tests/pull/324>
>>> >
>>>
>>>
>
>