Bug 62194 – new-run-webkit-tests: 'images' links on results.html don't work

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Ryosuke Niwa]

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.

[Ryosuke Niwa]

We need to modify
http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/test_result_writer.py

[Ojan Vafai]

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.

[Ryosuke Niwa]

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.

[WebKit Review Bot]

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.

[Ojan Vafai]

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.

[Dirk Pranke]

(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.

[Ryosuke Niwa]

(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.

[Ryosuke Niwa]

Created attachment 96821 [details]
Added _output_testname

The previous test also had a bug :(

[Ryosuke Niwa]

Comment on attachment 96821 [details]
Added _output_testname

Oops, forgot r?

[WebKit Review Bot]

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.

[Ryosuke Niwa]

Comment on attachment 96821 [details]
Added _output_testname

Clearing flags on attachment: 96821

Committed r88598: <http://trac.webkit.org/changeset/88598>

[Ryosuke Niwa]

All reviewed patches have been landed.  Closing bug.