Bug 57014 – New tests for CSS3: 2d-transforms

• Component: CSS
• Status: RESOLVED MOVED

[chereddy]

We have developed new tests for CSS3:2d-transforms as per the
W3C specification "http://www.w3.org/TR/css3-2d-transforms/".
I am proposing to add these new tests to webkit Layout tests.
Total number of tests:132

[chereddy]

Created attachment 86796 [details]
New tests for CSS3: 2d-transforms

[chereddy]

Created attachment 86938 [details]
New tests for CSS3: 2d-transforms 

New tests for CSS3: 2d-transforms. Use this patch file(name: css3-2d-transforms-tests.patch) for review.

[chereddy]

Created attachment 87336 [details]
css3-2d-transforms tests

In this patch, I replaced the old image with a new image and updated one test case with new image name. I created this new image(flower_yellow.png), so it can be checked into webkit along with the tests. No change in other tests.

[Eric Seidel (no email)]

Seems these tests will need results from at least one platform.

Are these taken verbatim from the w3c?

[Hajime Morrita]

This test suite might be a good candiate to reftests.
We can only test matrix by image, and make references for other tests using matrix.
What do you think?

http://trac.webkit.org/wiki/Writing%20Reftests

[Hayato Ito]

That's nice to use reftests in this case. But we should run tests on ports other than chromium, shouldn't we? If there is no such a strong urgent needs, I am +1 for reftests.

(In reply to comment #5)
> This test suite might be a good candiate to reftests.
> We can only test matrix by image, and make references for other tests using matrix.
> What do you think?
> 
> http://trac.webkit.org/wiki/Writing%20Reftests

[Laszlo Gombos]

(In reply to comment #4)
> Seems these tests will need results from at least one platform.
> 
> Are these taken verbatim from the w3c?

Eric, these tests were developed by Nokia. The intention is to cover the relevant W3C specification so that the test result would give an indication of how well a product confirms to the specification. We have plans to contribute the tests to W3C as well; a WebKit review should definitely help with the W3C process.

[chereddy]

(In reply to comment #4)
> Seems these tests will need results from at least one platform.

Eric, All 132 2d-transforms tests passed on below platforms:
  1. Mac with webkit
  2. Linux with QtWebkit

[Dirk Schulze]

Comment on attachment 87336 [details]
css3-2d-transforms tests

View in context: https://bugs.webkit.org/attachment.cgi?id=87336&action=review

This needs a lot of clean up.

> LayoutTests/css3/css3-2d-transforms/js/auto_functions.js:14
> +/*declared variables for expected values 
> + left difference, top difference, width difference, expected height difference */

Don't use multiline comments

> LayoutTests/css3/css3-2d-transforms/js/auto_functions.js:18
> +var exp_left_diff=0; 
> +var exp_top_diff=0;
> +var exp_width_diff=0; 
> +var exp_height_diff=0; 

spaces before and after the =
No underscores. This is against WebKit style.

> LayoutTests/css3/css3-2d-transforms/js/auto_functions.js:21
> +// Object initialV for initial values and object finalV for values after transformation

The indention of the following lines is wrong. Also, never use tabs.

> LayoutTests/css3/css3-2d-transforms/js/auto_functions.js:23
> +     var initialV = new Object();
> +     var finalV = new Object();	

please use self explaining names. What is the V for? use initialValues. Why not just using intital and final and so on?

> LayoutTests/css3/css3-2d-transforms/js/auto_functions.js:30
> +        // var result1 = document.getElementById ("result1");
> +	//     result1.innerHTML="<b>Before:</b> left:"+initialV.left+", top: "+initialV.top+", width:"+initialV.width+", height:" +initialV.height;  
> +       // calling applyTransform		 

Why is this commented out?

> LayoutTests/css3/css3-2d-transforms/js/auto_functions.js:40
> +	//   var result2 = document.getElementById ("result2");
> +	 //   result2.innerHTML="<b>After:</b> left:"+finalV.left+", top: "+finalV.top+", width:"+finalV.width+", height:" +finalV.height; 

Ditto, Don't check commented out code in.

> LayoutTests/css3/css3-2d-transforms/js/auto_functions.js:63
> +		/*2 pixels tolerance is given. We are validating difference in properties length. So 2 pixels positive side and 2 pixels in negative side are considered.  */ 		

Just use //

> LayoutTests/css3/css3-2d-transforms/js/auto_functions.js:89
> +		if (left1==1&&top1==1&&width1==1&&height1==1)			

This is general code used by more than one test. Please use WebKit style here.

> LayoutTests/css3/css3-2d-transforms/js/auto_functions.js:101
> +      var result=result1;

Ditto.

> LayoutTests/css3/css3-2d-transforms/js/auto_functions.js:113
> +          resultElement.innerHTML="FAIL";
> +          resultElement.style.background="red";

Ditto

> LayoutTests/css3/css3-2d-transforms/js/logResults.js:3
> +if (window.layoutTestController)
> +       { layoutTestController.dumpAsText();

put the brace behind the condition. Indention of all lines is wrong. Don't use tabs

> LayoutTests/css3/css3-2d-transforms/js/logResults.js:8
> +function notify()
> +	{

aline brace and function. Better to put the brace behind the function

> LayoutTests/css3/css3-2d-transforms/js/logResults.js:13
> + function logResults(result1){

space missing. What does result1 mean? take better names please

> LayoutTests/css3/css3-2d-transforms/js/logResults.js:14
> +      var result=result1;

Why are you creating a new var herer?

> LayoutTests/css3/css3-2d-transforms/js/logResults.js:19
> +	      if (result=="PASS"){
> +          resultElement.innerHTML="PASS";
> +          resultElement.style.background="green";

Spaces please

> LayoutTests/css3/css3-2d-transforms/js/logResults.js:26
> +          resultElement.innerHTML="FAIL";
> +          resultElement.style.background="red";
> +		  notify();

Ditto.

> LayoutTests/css3/css3-2d-transforms/tests/2d_transform_animation_3-expected.txt:1
> +Prev [index] Next

Wrong text coding

> LayoutTests/css3/css3-2d-transforms/tests/2d_transform_animation_3.html:1
> +<!DOCTYPE html>

indention is wrong

> LayoutTests/css3/css3-2d-transforms/tests/2d_transform_cssMatrix_attributes.html:28
> +//var display= document.getElementById("display");
> +//display.innerHTML="a: "+m1.a +"<br>"+"b: "+m1.b +"<br>"+"c: "+m1.c +"<br>"+"d: "+m1.d +"<br>"+"e: "+m1.e +"<br>"+"f: "+m1.f;
> +//calling validate() function

no out commented lines. I don't continue here. Just remaining patterns.

[Simon Fraser (smfr)]

These test should be submitted to the CSS test suite first for review. See http://wiki.csswg.org/test

[Brent Fulgham]

I believe these are all covered by CSS WPT now.