Bug 42938 - Output more descriptive PASS messages for list layout tests
Summary: Output more descriptive PASS messages for list layout tests
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Daniel Bates
URL:
Keywords:
Depends on: 42803
Blocks:
  Show dependency tree / graph
 
Reported: 2010-07-24 17:27 PDT by Daniel Bates
Modified: 2010-07-28 20:08 PDT (History)
1 user (show)

See Also:

Attachments
Patch (23.61 KB, patch)
2010-07-24 17:55 PDT, Daniel Bates 		no flags Details | Formatted Diff | Diff
Patch (51.39 KB, patch)
2010-07-25 12:23 PDT, Daniel Bates 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Bates 2010-07-24 17:27:58 PDT 
Following up on Darin Adler's comment in Bug #36734 <https://bugs.webkit.org/show_bug.cgi?id=36734#c14>,
"Tests that just write out "PASS PASS PASS" are suboptimal. Can you change this test so the output makes it clearer what is going on?", we should output more descriptive PASS messages for the list layout tests.
Comment 1 Daniel Bates 2010-07-24 17:55:17 PDT 
Created attachment 62514 [details]
Patch

I thought it would be easier to review the changes to the files LayoutTests/fast/lists/resources/dump-list.js, LayoutTests/fast/lists/w3-css3-list-styles-alphabetic-expected.txt, and LayoutTests/fast/lists/w3-css3-list-styles-numeric-expected.txt in a separate patch than to post an updated patch for Bug #42803.

My plan is to land Bug #42803 (as is). Then this bug (Bug #42938). Then, eventually Bug #36734.

I am open to suggestions on how to make the PASS messages more descriptive.
Notice, we only output the PASS message if the list item marker matches the list item text for each <li> in the list.

Without loss of generality, consider the following list:

<ol start="5">
    <li>5</li>
    <li>6</li>
</ol>

Then the PASS message would be: "PASS list markers matched list items for list that starts from 5 and has 2 items."

I thought to output the PASS message on a per list basis instead of a per <li> basis so as to make the output easier to read (since there would be less lines to read).
Comment 2 Darin Adler 2010-07-24 22:39:47 PDT 
Comment on attachment 62514 [details]
Patch

I guess this is OK. Better would be to include the strings of the markers that were tested. The danger of a test that compares two invisible things is that somehow they could both be wrong.
Comment 3 Daniel Bates 2010-07-24 22:58:24 PDT 
(In reply to comment #2)
> (From update of attachment 62514 [details])
> I guess this is OK. Better would be to include the strings of the markers that were tested. The danger of a test that compares two invisible things is that somehow they could both be wrong.

Would it be better to print PASS/FAIL for each <li>?

It would be verbose but it seems like this would be more beneficial.
Comment 4 Daniel Bates 2010-07-25 12:23:38 PDT 
Created attachment 62527 [details]
Patch

Updated patch to output PASS message for each <li>. Also, separated out common code in dump-list.js and LayoutTests/lists/w3-css3-list-styles-numeric.html into function testListItemMarkerEquals.
Comment 5 Daniel Bates 2010-07-28 20:08:39 PDT 
Comment on attachment 62527 [details]
Patch

Clearing flags on attachment: 62527

Committed r64259: <http://trac.webkit.org/changeset/64259>
Comment 6 Daniel Bates 2010-07-28 20:08:45 PDT 
All reviewed patches have been landed.  Closing bug.