Bug 37736

Summary: new-run-webkit-tests output is different from (and worse than) original run-webkit-tests
Product: WebKit Reporter: Maciej Stachowiak <mjs>
Component: Tools / TestsAssignee: Ojan Vafai <ojan>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, aroben, dpranke, eric, lforschler, mihaip, ojan, tony
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 34984    
Attachments:
Description Flags
Original run-webkit-tests results
none
new-run-webkit tests results for same set of tests, same failures
none
pretty patch diff
none
wdiff or the same 2 files
none
the new results.html format for new-run-webkit-tests none

Maciej Stachowiak
Reported 2010-04-16 17:14:26 PDT
new-run-webkit-tests output is different from (and worse than) original run-webkit-tests. The format is arbitrarily different, seems less conventiant, and has apparently broken links to wdiff output.
Attachments
Original run-webkit-tests results (1.30 KB, text/html)
2010-04-16 17:17 PDT, Maciej Stachowiak 		no flags
Details
new-run-webkit tests results for same set of tests, same failures (1.13 KB, text/html)
2010-04-16 17:19 PDT, Maciej Stachowiak 		no flags
Details
pretty patch diff (47.07 KB, text/html)
2010-04-19 03:11 PDT, Tony Chang 		no flags
Details
wdiff or the same 2 files (14.94 KB, text/html)
2010-04-19 03:12 PDT, Tony Chang 		no flags
Details
the new results.html format for new-run-webkit-tests (170.69 KB, image/png)
2011-04-19 14:26 PDT, Ojan Vafai 		no flags
Details
View All Add attachment proposed patch, testcase, etc.
Maciej Stachowiak
Comment 1 2010-04-16 17:17:33 PDT
Created attachment 53584 [details] Original run-webkit-tests results
Maciej Stachowiak
Comment 2 2010-04-16 17:19:06 PDT
Created attachment 53585 [details] new-run-webkit tests results for same set of tests, same failures
Adam Barth
Comment 3 2010-04-17 11:10:00 PDT
We should remove the wdiff links now that we have pretty patch wired up.
Eric Seidel (no email)
Comment 4 2010-04-18 22:27:32 PDT
(In reply to comment #3) > We should remove the wdiff links now that we have pretty patch wired up. So long as Chromium is OK with wdiff links being replaced by pretty diff links, that seems fine to me.
Tony Chang
Comment 5 2010-04-19 03:11:20 PDT
(In reply to comment #4) > (In reply to comment #3) > > We should remove the wdiff links now that we have pretty patch wired up. > > So long as Chromium is OK with wdiff links being replaced by pretty diff links, > that seems fine to me. I think wdiff is a little bit easier to read in the case of font only differences, but not so much that I think we should keep it. Instead, we should try to make pretty patch handle font only differences better. I'll upload some sample diffs to demonstrate.
Tony Chang
Comment 6 2010-04-19 03:11:57 PDT
Created attachment 53665 [details] pretty patch diff
Tony Chang
Comment 7 2010-04-19 03:12:27 PDT
Created attachment 53666 [details] wdiff or the same 2 files
Eric Seidel (no email)
Comment 8 2010-04-19 03:44:21 PDT
CCing Adam Roben, the original PrettyPatch author.
Eric Seidel (no email)
Comment 9 2010-04-19 03:45:32 PDT
That's really nice btw. I'm not sure we really want to kill wdiff. We shouldn't show the "wdiff" link on platforms which don't have wdiff installed however. :)
Ojan Vafai
Comment 10 2010-04-19 07:20:57 PDT
(In reply to comment #9) > That's really nice btw. I'm not sure we really want to kill wdiff. We > shouldn't show the "wdiff" link on platforms which don't have wdiff installed > however. :) Ideally we'd use wdiff it's available and pretty-patch otherwise, since wdiff output is a bit better. I don't think there are any cases where you actually want both.
Adam Roben (:aroben)
Comment 11 2010-04-19 07:45:35 PDT
(In reply to comment #10) > (In reply to comment #9) > > That's really nice btw. I'm not sure we really want to kill wdiff. We > > shouldn't show the "wdiff" link on platforms which don't have wdiff installed > > however. :) > > Ideally we'd use wdiff it's available and pretty-patch otherwise, since wdiff > output is a bit better. I don't think there are any cases where you actually > want both. We could change PrettyPatch to create wdiff-like output in the cases where it currently shows intra-line diffs. Maybe we'd want this to be a special mode for PrettyPatch, though, as there are some cases where I think that would be undesirable (e.g., when changing a comment in C++ code and re-wrapping the comment lines).
Dirk Pranke
Comment 12 2011-04-17 15:00:34 PDT
Ojan, I'm punting this one to you for now. Feel free to punt it back if you don't want it.
Ojan Vafai
Comment 13 2011-04-19 14:26:33 PDT
Created attachment 90253 [details] the new results.html format for new-run-webkit-tests
Ojan Vafai
Comment 14 2011-04-19 14:28:33 PDT
As of r84294, I think we can call this resolved. Please let me know (or file new bugs) if there are still concerns with the output format.
Note You need to log in before you can comment on or make changes to this bug.