[Webkit-unassigned] [Bug 60957] Pass flags from run_webkit_tests to ImageDiff, read results from stdout and store in unexpected_results.json

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=60957





--- Comment #6 from Dirk Pranke <dpranke at chromium.org>  2011-05-31 14:39:17 PST ---
(In reply to comment #5)
> > 
> > > Tools/Scripts/webkitpy/layout_tests/layout_package/manager.py:161
> > > +        port_obj.add_port_test_data(result_type, test, test_dict)
> > 
> > Can you add a FIXME here to extract this data from the failure_types list directly instead of having to jump through the handle_port_finished_test / add_port_test_data hoop instead?
> > 
> > If it's not clear what I have in mind, I can upload a modified version of this patch that does what I'm talking about.
> > 
> > > Tools/Scripts/webkitpy/layout_tests/layout_package/worker.py:143
> > >          self._worker_connection.post_message('finished_test', result, elapsed_time)
> > 
> > Can we change this so that instead of sending a port_finished_test message we call into a port routine that returns a dict and send the dict along in finished_test? I don't really want to double the number of messages sent in a test run, and I have no concerns over changing the message format.
> 
> Part 1 is not 100% clear to me, unless part 2 is tied into it - I should be able to upload what I think you mean tomorrow. It'll be:
>  - new field on TestResult object
>  - new routine on Port called from worker thread to add arbitrary data to TestResult
>  - Port.add_port_test_data replaced by a generic routine that checks for above arbitrary data on the TestResult object and, if present, adds to test_dict
>  - Get rid of the port_finished_test send & receive code.
>  - Get rid of the dependency on test_writer.
> 

Sounds pretty close :)

> > > Tools/Scripts/webkitpy/layout_tests/port/chromium.py:196
> > > +            if self.get_option('write_image_diff_metrics'):
> > 
> > Is there a reason we shouldn't just always pass the flag and write the metrics?
> 
> Tony Chang & I talked for a while when he first reviewed 60569, and it was important to him not to slow down NRWT on the bots; especially once we start computing multiple metrics over the images (qv discussion of 60964), that's multiple passes over every pixel of every difference image, and a lot of extra data being written to unexpected_results.json and archived.
> 

Well, given that files that fail the image check are a very small percentage of the total number of tests we run, I'd be surprised if it made much difference.

> As long as the only consumer of this data is RebaselineServer, it seemed reasonable to make it argument-driven and only happen on interactive local runs, although now thinking about it you might at times want it from a bot if there's a bug you can't reproduce locally?

That, or eventually we'll build tools where the gardeners (or, any webkit committer looking at failures in the tree) can also pull this data to help them determine how to triage failures.

That said, I'm happy to start with a flag and we can get rid of it later.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.