Bug 35778 - new-run-webkit-tests: fix --clobber-old-results
Summary: new-run-webkit-tests: fix --clobber-old-results
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Dirk Pranke
Depends on:
Reported: 2010-03-04 19:57 PST by Dirk Pranke
Modified: 2010-03-09 13:23 PST (History)
2 users (show)

See Also:

Patch (1.69 KB, patch)
2010-03-04 19:57 PST, Dirk Pranke
no flags Details | Formatted Diff | Diff
change 'path_utils.' to 'port_obj.' (1.73 KB, patch)
2010-03-09 11:58 PST, Dirk Pranke
japhet: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dirk Pranke 2010-03-04 19:57:26 PST
new-run-webkit-tests: fix --clobber-old-results
Comment 1 Dirk Pranke 2010-03-04 19:57:40 PST
Created attachment 50082 [details]
Comment 2 Tony Chang 2010-03-04 20:00:56 PST
I'm not a reviewer, but LGTM.
Comment 3 Eric Seidel (no email) 2010-03-05 12:46:59 PST
Comment on attachment 50082 [details]

I'm confused by this change.  What directories are we rm -rf-ing?  Does Chromium store results separate from the actual test files?  webkit.org does not.  Directories like platform/mac may contain both results and additional mac-specific tests.

FYI, there is a similar (same?) option on run-webkit-tests called --reset-results, which runs the tests like normal, except tricks the test engine into thinking that every result is new and causes it to overwrite all -expected.txt files in their proper places.
Comment 4 Dirk Pranke 2010-03-05 13:25:34 PST
This deletes the results of the test run (files in layout-test-results/) , not the actual baselines. For some reason Chromium doesn't do this by default every time, and so things like the rebaseline tool may get confused and pull old stale results.

Tony, is there some reason we don't just make this option true by default (or just do it every time)?
Comment 5 Tony Chang 2010-03-07 04:19:21 PST
(In reply to comment #4)
> Tony, is there some reason we don't just make this option true by default (or
> just do it every time)?

run-webkit-tests doesn't delete results every time either:

As far as I can tell, it doesn't have an option to delete the results.  But the buildbot slaves on build.webkit.org must delete results between runs because you can download the failing tests from the waterfall.  Maybe it's done in by the slave rather than by run-webkit-tests.  I don't have a preference for which script does this.
Comment 6 Dimitri Glazkov (Google) 2010-03-08 15:19:14 PST
Comment on attachment 50082 [details]

Comment 7 Dirk Pranke 2010-03-08 15:57:04 PST
Committed r55689: <http://trac.webkit.org/changeset/55689>
Comment 8 Michael Nordman 2010-03-08 21:45:02 PST
This patch was reverted, path_utils.py is nowhere to be found upstream so the canary bots weren't running the layout tests at all. (And heaven forbid we roll that into view).

Dumi helped me do the revert since I'm not a webkit committer.
See https://trac.webkit.org/changeset/55709
Comment 9 Dirk Pranke 2010-03-08 21:50:52 PST
Thanks for catching this. Apparently I didn't actually test this upstream, as it wouldn't have executed. Very sloppy :(
Comment 10 Dirk Pranke 2010-03-09 11:58:55 PST
Created attachment 50337 [details]
change 'path_utils.' to 'port_obj.'

fix bad merge from downstream - rename 'path_utils.' to 'port_obj.'
Comment 11 Nate Chapin 2010-03-09 13:15:48 PST
Comment on attachment 50337 [details]
change 'path_utils.' to 'port_obj.'

Comment 12 Nate Chapin 2010-03-09 13:18:45 PST
Comment on attachment 50337 [details]
change 'path_utils.' to 'port_obj.'

rs=me for real this time, apparently I can't operate a dropdown menu.
Comment 13 Dirk Pranke 2010-03-09 13:23:43 PST
Committed r55741: <http://trac.webkit.org/changeset/55741>