RE: Status of review for PR-324

Thanks for the feedback, everyone. I've passed this on to our test engineer and we'll work to get these updated.

-Jacob


On Tue, Oct 29, 2013 at 8:31 PM, Rick Byers <rbyers@google.com> wrote:
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>
[PR324-Mirror] <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>

Received on Thursday, 31 October 2013 17:51:50 UTC