WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
37736
new-run-webkit-tests output is different from (and worse than) original run-webkit-tests
https://bugs.webkit.org/show_bug.cgi?id=37736
Summary
new-run-webkit-tests output is different from (and worse than) original run-w...
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.
Top of Page
Format For Printing
XML
Clone This Bug