Reproduction steps: 1. Open http://build.webkit.org/results/Chromium%20Mac%20Release%20(Tests)/r88219%20(7942)/results.html 2. Click any 'images' Expected result: Actual and expected images are shown. Actual result: No images are shown. The problem is that nrwt's diffs.html contain absolute path to local files instead of relatives paths: See http://build.webkit.org/results/Chromium%20Mac%20Release%20(Tests)/r88219%20(7942)/css2.1/t1508-c527-font-05-b-diffs.html for example.
We need to modify http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/test_result_writer.py
We should use the same trick we use on the results.html page (http://trac.webkit.org/browser/trunk/LayoutTests/fast/harness/results.html#L366). If it's a file URL, we load the full path, if it's not, load the relative path.
Created attachment 96808 [details] fixes the bug I don't know if I should be adding a test or where I should be adding a test since test_result_writer.py doesn't seem to have any tests.
Attachment 96808 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Tools/ChangeLog', u'Tools/Scripts/webkitpy..." exit_code: 1 Tools/Scripts/webkitpy/layout_tests/layout_package/test_result_writer.py:247: whitespace after '{' [pep8/E201] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 96808 [details] fixes the bug View in context: https://bugs.webkit.org/attachment.cgi?id=96808&action=review > Tools/Scripts/webkitpy/layout_tests/layout_package/test_result_writer.py:119 > + def output_filename(self, modifier, local=False): I'd rather you just create an output_testname method. I think that results in better self-documenting code.
(In reply to comment #3) > Created an attachment (id=96808) [details] > fixes the bug > > I don't know if I should be adding a test or where I should be adding a test since test_result_writer.py doesn't seem to have any tests. Hm. That's a bit odd. Feel free to start writing some in a test_result_writer_unittest.py class, but I probably wouldn't R- you for this. (In reply to comment #5) > (From update of attachment 96808 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=96808&action=review > > > Tools/Scripts/webkitpy/layout_tests/layout_package/test_result_writer.py:119 > > + def output_filename(self, modifier, local=False): > > I'd rather you just create an output_testname method. I think that results in better self-documenting code. Agreed.
(In reply to comment #6) > (In reply to comment #3) > > Created an attachment (id=96808) [details] [details] > > fixes the bug > > > > I don't know if I should be adding a test or where I should be adding a test since test_result_writer.py doesn't seem to have any tests. > > Hm. That's a bit odd. Feel free to start writing some in a test_result_writer_unittest.py class, but I probably wouldn't R- you for this. Okay. I actually don't know how I can test that local path isn't included in the generated html.
Created attachment 96821 [details] Added _output_testname The previous test also had a bug :(
Comment on attachment 96821 [details] Added _output_testname Oops, forgot r?
Attachment 96821 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Tools/ChangeLog', u'Tools/Scripts/webkitpy..." exit_code: 1 Tools/Scripts/webkitpy/layout_tests/layout_package/test_result_writer.py:248: whitespace after '{' [pep8/E201] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 96821 [details] Added _output_testname Clearing flags on attachment: 96821 Committed r88598: <http://trac.webkit.org/changeset/88598>
All reviewed patches have been landed. Closing bug.