681615 - Move a couple of tests from content/html/content/test to content/html/content/test/forms

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Patch v1

I ran into those yesterday; I thought you might this.

Comment 1

[Mounir Lamouri (:mounir)]

Comment on attachment 555366 [details] [diff] [review]
Patch v1

Review of attachment 555366 [details] [diff] [review]:
-----------------------------------------------------------------

r=mounir, if you fix the things I pointed or open follow-ups.

::: content/html/content/test/forms/Makefile.in
@@ +60,5 @@
>  		test_output_element.html \
>  		test_button_attributes_reflection.html \
>  		test_textarea_attributes_reflection.html \
> +		test_validation.html \
> +		test_validation_tooLong.html \

I don't like those tests (yes, I wrote them...). I don't think there is anything we could do about them except review them and see what could be improved/fixed.
BTW, why test_validation_tooLong.html and not test_maxlength_attribute.html?

@@ +62,5 @@
>  		test_textarea_attributes_reflection.html \
> +		test_validation.html \
> +		test_validation_tooLong.html \
> +		test_datalist_options_attribute-1.html \
> +		test_datalist_options_attribute-2.html \

Could you merge them?
Maybe we should make sure the attribute reflection part is in another test too and we should name the test "test_datalist_element.html" because I don't see anything else to test.

Comment 2

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Patch v2

(In reply to Mounir Lamouri (:volkmar) from comment #1)
> Comment on attachment 555366 [details] [diff] [review]
> Patch v1
> 
> Review of attachment 555366 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r=mounir, if you fix the things I pointed or open follow-ups.
> 
> ::: content/html/content/test/forms/Makefile.in
> @@ +60,5 @@
> >  		test_output_element.html \
> >  		test_button_attributes_reflection.html \
> >  		test_textarea_attributes_reflection.html \
> > +		test_validation.html \
> > +		test_validation_tooLong.html \
> 
> I don't like those tests (yes, I wrote them...). I don't think there is
> anything we could do about them except review them and see what could be
> improved/fixed.

Bug 681642

> BTW, why test_validation_tooLong.html and not test_maxlength_attribute.html?

Fixed

> @@ +62,5 @@
> >  		test_textarea_attributes_reflection.html \
> > +		test_validation.html \
> > +		test_validation_tooLong.html \
> > +		test_datalist_options_attribute-1.html \
> > +		test_datalist_options_attribute-2.html \
> 
> Could you merge them?

Sure.

> Maybe we should make sure the attribute reflection part is in another test
> too and we should name the test "test_datalist_element.html" because I don't
> see anything else to test.

Which attribute reflection part?

Comment 3

[Mounir Lamouri (:mounir)]

Comment on attachment 555397 [details] [diff] [review]
Patch v2

Review of attachment 555397 [details] [diff] [review]:
-----------------------------------------------------------------

function checkClassesAndAttributes checks if 'options' is an IDL attribute of HTMLDataListElement.
Do we want to keep that or move it to an attribute reflection test?

r=mounir

Comment 4

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

No, that's just because it's an IDL attribute, nothing to do with reflection.

Comment 5

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

http://hg.mozilla.org/mozilla-central/rev/80db03383c34