RE: Request for Review: single-touch tests; deadline Sept 24

First of all, +1 to continuing.

A couple comments on the type-checking code
1. I noticed you used
	assert_equals(Object.prototype.toString.call(ev[name]), "[object
TouchList]", name + " attribute of type TouchList");
instead of the I-assume-is-equivalent
	assert_true(ev[name] instanceof TouchList, name + " attribute of
type TouchList");
to check whether the said attribute is a TouchList object. While the latter
one results in the "Can't find variable: TouchList" error in WebKit [1], the
former works without a hitch. We might be inadvertently masking the WebKit
bug if we use the former syntax. (Note that with the type-checking code in
place, the line that initially exposed that WebKit bug (line 134 in your
version) can and should be removed, leaving the above line as the only place
which would expose the bug.)
2. The number of test assertions exploded from 10+ to 80+ with the
type-checking code (with many of them repeating multiple times as the
functions are called at different places). I would suggest that instead of
having each assertion in its own little test(), we wrap the content of the
entire check_touch_attrs() function (similarly for check_touchevent_attrs())
with a single test(), with a broad test name of something like "Touch
attributes exist and are of the required types." One downside I can think of
is that only the first failing assertion within the entire function will be
reported. I recognize that this might well just be a matter of style and
personal preference. So this is just an idea to consider, not really a
complaint.

Regards, Cathy.
P.S. I can send you the version that I've been messing with to achieve #1
and #2.

[1]
http://www.w3.org/mid/CAFUtAY-cMGHH0uzcNB3tA5cQFXJWXM5-Jxr+ziMVUnbL1g5NPA@ma
il.gmail.com


> -----Original Message-----
> From: Barstow Art (Nokia-CIC/Boston)
> Sent: Monday, September 17, 2012 5:34 PM
> To: public-webevents@w3.org
> Subject: Re: Request for Review: single-touch tests; deadline Sept 24
> 
> On 9/13/12 5:39 PM, Chan Cathy (Nokia-CIC/Boston) wrote:
> > Hi all,
> >
> > I checked the single-touch tests against the test assertions that I
> > wrote up a while ago
> > (http://www.w3.org/2010/webevents/wiki/TestAssertions, also updated
> to
> > add a few assertions that I missed) and can report that the coverage was
> pretty good. A few specific comments below.
> >
> > 1. In most cases, the existence of attributes in the object and event
> > interfaces are verified (assertions 1.1.2, 1.2.1, 1.3.1.1, etc),
> > except for a. Touch interface: identifier and target b. touchstart
> > event: changedTouches, targetTouches c. touchmove event:
> > changedTouches, targetTouches, altKey, ctrlKey, metaKey, shiftKey d.
> > touchend event: changedTouches, targetTouches, altKey, ctrlKey,
> > metaKey, shiftKey Note that in some cases, existence is indirectly
> > verified by accessing the attributes.
> >
> > 2. In all cases, the data types of the attributes in the object and
> > event interfaces are not verified (assertions 1.1.3, 1.2.2, 1.3.1.2,
etc).
> 
> I  hacked on a version of Matt's single-touch tests to address the two
items
> above (and the one typo mentioned below) [1]. Based on some recent audio
> tests for HTML, I created separate functions to check the existence and
types
> of the TouchEvent and Touch interfaces. I don't know how frequently these
> checks need to be called. Perhaps just once (for all event types) or for
some
> set of the start, move and end events (currently, some of this type of
> validation is done in multiple event callbacks).
> 
> If there is interest in me continuing this version, I can work on at least
some
> of the other items below. I can throw my tests away too, f.ex, if someone
> wants to address the various gaps directly in Matt's version. Regardless,
> comments/flames re [1] are welcome, especially with the type checking
code.
> 
> -Thanks, AB
> 
> [1] <https://raw.github.com/AFBarstow/WebEvents/master/single-
> touch.html>
> 
> > 3. Assertion 1.2.5, which verifies that each item of a TouchList
> > object is a Touch object, is not covered.
> >
> > 4. Assertions 1.3.3.9 and 1.4.3.8, which verify that the target of
> > each Touch object in targetTouches of the touchstart and touchmove
> > event matches the element that receives the event, is not covered.
> >
> > 5. Similarly, assertion 1.5.2.6, which verifies that the target of at
> > least one Touch object in changedTouches of the touchend/touchcancel
> > event matches the element that receives the event, is not covered.
> >
> > 6. Assertion 1.6.2, which verifies that the target of the item in
> > changedTouches of the touchstart event matches the element that
> > receives the event, is not covered.
> >
> > These all seem reasonably easy to add to the test file.
> >
> > Also, as Art mentioned before, there are no tests on the touchcancel
> > event or the DOM interfaces for creating Touch and TouchList objects
> > (note that I haven't included any assertions for those in the wiki
either).
> >
> > I also found one typo in line 91 in the touchmove section:
> > s/touchend/touchmove/
> >
> > Regards, Cathy.
> 
> 

Received on Thursday, 20 September 2012 21:55:11 UTC