Bug 80709

Created attachment 131079 [details] Patch

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.

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.

It's interesting that the result came out much clearer in the bugzilla comments.

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

...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 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.

Ready for review

It might make sense to move testharness.js to resources/w3c/ to signify the fact it's imported from w3c repository.

Comment on attachment 132376 [details] Patch Clearing flags on attachment: 132376 Committed r111411: <http://trac.webkit.org/changeset/111411>

All reviewed patches have been landed. Closing bug.