Re: Submitted tests

On 06/14/2011 01:11 PM, Ms2ger wrote:
> Hi James,
>
> On 06/07/2011 11:22 PM, James Graham wrote:
>> I just commited some Opera test submissions for the classList attribute,
>> the history pushstate API and the time element.
>>
>
> I've looked over the classList test. I believe it should be approved,
> with the following nits addressed:
>
>  > assert_true( elem.classList.constructor == window.DOMTokenList );
>  > assert_true( getComputedStyle(elem,null).fontStyle != 'italic',
>  > 'critical test; required by the testsuite' );
>> assert_true( getComputedStyle(elem,null).fontStyle == 'italic',
>  > 'critical test; required by the testsuite' );
>  > assert_false( getComputedStyle(elem,null).fontStyle == 'italic' );
>  > assert_true( getComputedStyle(elem,null).fontStyle == 'italic' );
>  > assert_false( getComputedStyle(elem,null).fontStyle == 'italic' );
>
> Should use assert_equals and assert_not_equals.

Fixed.

>> /* the normative part of the spec states that:
>> "unless the length is zero, in which case there are no supported
>> property indices"
>> ...
>> "The term[...] supported property indices [is] used as defined in the
>> WebIDL specification."
>> WebIDL creates actual OwnProperties and then [] just acts as a normal
>> property lookup */
>> assert_equals( elem.classList[2], window.undefined );
>
> The quote was copied too eagerly; the length isn't zero.

I don't quite follow this.

> As a general comment, I'd like s/window.undefined/undefined/g, as the
> former looks rather silly.

Agreed. IIRC the test author has some explanation involving legacy 
browsers, but I changed it anyway.

> Also, I'd like to see tests for classList.toString called explicitly,
> rather than through |classList + ""|,

Fixed.

and for setting the readonly
> properties in ES5 strict mode.

Agreed, but that's out of scope for this submission.

Received on Tuesday, 26 July 2011 09:45:54 UTC