Bug 80709 - Convert regions parsing test to use testharness.js
Summary: Convert regions parsing test to use testharness.js
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh Intel OS X 10.7
: P2 Normal
Assignee: Jacob Goldstein
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-03-09 10:25 PST by Jacob Goldstein
Modified: 2012-03-20 10:56 PDT (History)
7 users (show)

See Also:


Attachments
Patch (73.01 KB, patch)
2012-03-09 12:52 PST, Jacob Goldstein
no flags Details | Formatted Diff | Diff
Patch (73.36 KB, patch)
2012-03-16 14:17 PDT, Jacob Goldstein
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jacob Goldstein 2012-03-09 10:25:20 PST
Update test to use testharness.js for synergy between W3C and WebKit test style
Comment 1 Jacob Goldstein 2012-03-09 12:52:28 PST
Created attachment 131079 [details]
Patch
Comment 2 Adam Barth 2012-03-09 15:06:34 PST
Comment on attachment 131079 [details]
Patch

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

> LayoutTests/fast/regions/webkit-flow-parsing-expected.txt:26
> +Result	Test Name	Message
> +Pass	testParse: Assigned none to property, expected return = none	
> +Pass	testParse: Assigned first-flow to property, expected return = first-flow	
> +Pass	testParse: Assigned 'first-flow' to property, expected return = ''	
> +Pass	testParse: Assigned ; to property, expected return = ''	
> +Pass	testParse: Assigned 1 to property, expected return = ''	
> +Pass	testParse: Assigned 1.2 to property, expected return = ''	
> +Pass	testParse: Assigned -1 to property, expected return = ''	
> +Pass	testParse: Assigned 12px to property, expected return = ''	
> +Pass	testComputedStyle: Assigned none to property, expected return = none	
> +Pass	testComputedStyle: Assigned \"\" to property, expected return = none	
> +Pass	testComputedStyle: Assigned 'first-flow' to property, expected return = none	
> +Pass	testComputedStyle: Assigned first-flow to property, expected return = first-flow	
> +Pass	testComputedStyle: Assigned 12px to property, expected return = none	
> +Pass	testNotInherited: Assigned none, none to property, expected return = none	
> +Pass	testNotInherited: Assigned none, child-flow to property, expected return = child-flow	
> +Pass	testNotInherited: Assigned parent-flow, none to property, expected return = none	
> +Pass	testNotInherited: Assigned parent-flow, child-flow to property, expected return = child-flow	

This is hard to read in a text dump.  Did the tests pass or fail/  The Result column appears to be empty.  Given that text dumps is the main way we look at these results, I'm not sure this is an improvement.
Comment 3 Jacob Goldstein 2012-03-09 15:50:25 PST
I understand your concern.  Some of this is user defined, and the output could also be improved via customizations to the testharnessreport.js file.

The message you see after "Pass", for example, "testParse: Assigned none to property, expected return = none", is the user defined string from the description argument of the assert_equals method.   assert_equals looks like this:

assert_equals(actual, expected, description)

All of the methods in testharness.js include a description argument that is output after the test result.  In the W3C test suite, the testharness.css file formats the output into a slightly more readable output.  Since we are dumping as text, that isn't possible, but other customizations in testharnessreport.js are.  I will look into this further.




(In reply to comment #2)
> (From update of attachment 131079 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=131079&action=review
> 
> > LayoutTests/fast/regions/webkit-flow-parsing-expected.txt:26
> > +Result	Test Name	Message
> > +Pass	testParse: Assigned none to property, expected return = none	
> > +Pass	testParse: Assigned first-flow to property, expected return = first-flow	
> > +Pass	testParse: Assigned 'first-flow' to property, expected return = ''	
> > +Pass	testParse: Assigned ; to property, expected return = ''	
> > +Pass	testParse: Assigned 1 to property, expected return = ''	
> > +Pass	testParse: Assigned 1.2 to property, expected return = ''	
> > +Pass	testParse: Assigned -1 to property, expected return = ''	
> > +Pass	testParse: Assigned 12px to property, expected return = ''	
> > +Pass	testComputedStyle: Assigned none to property, expected return = none	
> > +Pass	testComputedStyle: Assigned \"\" to property, expected return = none	
> > +Pass	testComputedStyle: Assigned 'first-flow' to property, expected return = none	
> > +Pass	testComputedStyle: Assigned first-flow to property, expected return = first-flow	
> > +Pass	testComputedStyle: Assigned 12px to property, expected return = none	
> > +Pass	testNotInherited: Assigned none, none to property, expected return = none	
> > +Pass	testNotInherited: Assigned none, child-flow to property, expected return = child-flow	
> > +Pass	testNotInherited: Assigned parent-flow, none to property, expected return = none	
> > +Pass	testNotInherited: Assigned parent-flow, child-flow to property, expected return = child-flow	
> 
> This is hard to read in a text dump.  Did the tests pass or fail/  The Result column appears to be empty.  Given that text dumps is the main way we look at these results, I'm not sure this is an improvement.
Comment 4 Adam Barth 2012-03-09 16:13:59 PST
It's interesting that the result came out much clearer in the bugzilla comments.
Comment 5 Jacob Goldstein 2012-03-16 14:17:10 PDT
Created attachment 132376 [details]
Patch

Updated to format out from testharness.js such that an HTML table is no longer used and the text file produced by dumpAsText is readable
Comment 6 Jacob Goldstein 2012-03-16 14:22:52 PDT
...for format out[put] from...

(In reply to comment #5)
> Created an attachment (id=132376) [details]
> Patch
> 
> Updated to format out from testharness.js such that an HTML table is no longer used and the text file produced by dumpAsText is readable
Comment 7 Adam Barth 2012-03-16 14:32:01 PDT
Comment on attachment 132376 [details]
Patch

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

> LayoutTests/fast/regions/webkit-flow-parsing-expected.txt:19
> +PASS Test Parse none 
> +PASS Test Parse first-flow 
> +PASS Test Parse 'first-flow' 
> +PASS Test Parse ; 
> +PASS Test Parse 1 
> +PASS Test Parse 1.2 
> +PASS Test Parse -1 
> +PASS Test Parse 12px 
> +PASS Test Computed Style none 
> +PASS Test Computed Style '' 
> +PASS Test Computed Style 'first-flow' 
> +PASS Test Computed Style first-flow 
> +PASS Test Computed Style 12px  
> +PASS Test Non-Inherited none, none 
> +PASS Test Non-Inherited none, child-flow 
> +PASS Test Non-Inherited parent-flow, none 
> +PASS Test Non-Inherited parent-flow, child-flow 

This output is much better.  Thanks.
Comment 8 Jacob Goldstein 2012-03-20 09:54:19 PDT
Ready for review
Comment 9 Ryosuke Niwa 2012-03-20 10:26:39 PDT
It might make sense to move testharness.js to resources/w3c/ to signify the fact it's imported from w3c repository.
Comment 10 WebKit Review Bot 2012-03-20 10:56:32 PDT
Comment on attachment 132376 [details]
Patch

Clearing flags on attachment: 132376

Committed r111411: <http://trac.webkit.org/changeset/111411>
Comment 11 WebKit Review Bot 2012-03-20 10:56:38 PDT
All reviewed patches have been landed.  Closing bug.