Bug 35778 – new-run-webkit-tests: fix --clobber-old-results

• Component: New Bugs
• Status: RESOLVED FIXED

[Dirk Pranke]

new-run-webkit-tests: fix --clobber-old-results

[Dirk Pranke]

Created attachment 50082 [details]
Patch

[Tony Chang]

I'm not a reviewer, but LGTM.

[Eric Seidel (no email)]

Comment on attachment 50082 [details]
Patch

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.

[Dirk Pranke]

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)?

[Tony Chang]

(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:
http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/run-webkit-tests#L997

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.

[Dimitri Glazkov (Google)]

Comment on attachment 50082 [details]
Patch

rs=me.

[Dirk Pranke]

Committed r55689: <http://trac.webkit.org/changeset/55689>

[Michael Nordman]

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

[Dirk Pranke]

Thanks for catching this. Apparently I didn't actually test this upstream, as it wouldn't have executed. Very sloppy :(

[Dirk Pranke]

Created attachment 50337 [details]
change 'path_utils.' to 'port_obj.'

fix bad merge from downstream - rename 'path_utils.' to 'port_obj.'

[Nate Chapin]

Comment on attachment 50337 [details]
change 'path_utils.' to 'port_obj.'

rs=me

[Nate Chapin]

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.

[Dirk Pranke]

Committed r55741: <http://trac.webkit.org/changeset/55741>