Bug 77685 - Ref Tests should support plain SVG files
Summary: Ref Tests should support plain SVG files
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-02-02 14:49 PST by Dirk Schulze
Modified: 2012-02-03 19:52 PST (History)
6 users (show)

See Also:


Attachments
fixes the bug (7.83 KB, patch)
2012-02-03 03:21 PST, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
fixes the bug (9.62 KB, patch)
2012-02-03 12:08 PST, Ryosuke Niwa
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dirk Schulze 2012-02-02 14:49:14 PST
Ref Tests should support plain SVG files with the ending .svg. It would be great to have them for SVG tests as well.
Comment 1 Hayato Ito 2012-02-02 18:03:12 PST
That sounds reasonable.

My initial thought is if we have foo.svg and foo-expected.svg, we should consider it as a reftest.
Is this a feasible idea?
Comment 2 Ryosuke Niwa 2012-02-02 18:06:32 PST
(In reply to comment #1)
> That sounds reasonable.
> 
> My initial thought is if we have foo.svg and foo-expected.svg, we should consider it as a reftest.
> Is this a feasible idea?

Yes, that sounds reasonable. As I said on IRC, we just need to modify functions in base.py that find reference files.
Comment 3 Ryosuke Niwa 2012-02-03 03:10:04 PST
A patch coming...
Comment 4 Ryosuke Niwa 2012-02-03 03:21:31 PST
Created attachment 125301 [details]
fixes the bug
Comment 5 Tony Chang 2012-02-03 11:23:08 PST
Comment on attachment 125301 [details]
fixes the bug

View in context: https://bugs.webkit.org/attachment.cgi?id=125301&action=review

> Tools/Scripts/webkitpy/layout_tests/port/base.py:472
> +            for (expectation, prefix) in [('==', ''), ('!=', '-mismatch')]:

Nit: Remove unnecessary () and use () instead of []:
  for expectation, prefix in (('==', ''), ('!=', '-mismatch')):

> Tools/Scripts/webkitpy/layout_tests/port/base.py:482
> +        return any(self.reference_files(test_name))

Do you need any() here?  Don't you just want to see if the return value is not empty?
Comment 6 Ryosuke Niwa 2012-02-03 12:00:35 PST
Comment on attachment 125301 [details]
fixes the bug

View in context: https://bugs.webkit.org/attachment.cgi?id=125301&action=review

>> Tools/Scripts/webkitpy/layout_tests/port/base.py:472
>> +            for (expectation, prefix) in [('==', ''), ('!=', '-mismatch')]:
> 
> Nit: Remove unnecessary () and use () instead of []:
>   for expectation, prefix in (('==', ''), ('!=', '-mismatch')):

Okay, will fix.

>> Tools/Scripts/webkitpy/layout_tests/port/base.py:482
>> +        return any(self.reference_files(test_name))
> 
> Do you need any() here?  Don't you just want to see if the return value is not empty?

Oh oops, you're right. No need.
Comment 7 Ryosuke Niwa 2012-02-03 12:08:09 PST
I'm gonna just get rid of is_reftest. It's a useless function now.
Comment 8 Ryosuke Niwa 2012-02-03 12:08:47 PST
Created attachment 125379 [details]
fixes the bug
Comment 9 Ryosuke Niwa 2012-02-03 13:48:40 PST
Comment on attachment 125379 [details]
fixes the bug

Clearing flags on attachment: 125379

Committed r106692: <http://trac.webkit.org/changeset/106692>
Comment 10 Ryosuke Niwa 2012-02-03 13:48:45 PST
All reviewed patches have been landed.  Closing bug.
Comment 11 James Robinson 2012-02-03 19:31:05 PST
This broke webkitpy tests on chromium win:

http://build.webkit.org/builders/Chromium%20Win%20Release%20%28Tests%29/builds/23418/steps/webkitpy-test/logs/stdio

FAIL: test_reference_files (webkitpy.layout_tests.port.base_unittest.PortTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "E:\b\chromium-win-release-tests\build\Tools\Scripts\webkitpy\layout_tests\port\base_unittest.py", line 360, in test_reference_files
    self.assertEqual(port.reference_files('passes/svgreftest.svg'), [('==', '/test.checkout/LayoutTests/passes/svgreftest-expected.svg')])
AssertionError: [('==', 'c:/test.checkout/LayoutTests/passes/svgreftest-expected.svg')] != [('==', '/test.checkout/LayoutTests/passes/svgreftest-expected.svg')]

----------------------------------------------------------------------
Comment 12 Ryosuke Niwa 2012-02-03 19:33:38 PST
(In reply to comment #11)
> This broke webkitpy tests on chromium win:
> 
> http://build.webkit.org/builders/Chromium%20Win%20Release%20%28Tests%29/builds/23418/steps/webkitpy-test/logs/stdio
> 
> FAIL: test_reference_files (webkitpy.layout_tests.port.base_unittest.PortTest)
> ----------------------------------------------------------------------
> Traceback (most recent call last):
>   File "E:\b\chromium-win-release-tests\build\Tools\Scripts\webkitpy\layout_tests\port\base_unittest.py", line 360, in test_reference_files
>     self.assertEqual(port.reference_files('passes/svgreftest.svg'), [('==', '/test.checkout/LayoutTests/passes/svgreftest-expected.svg')])
> AssertionError: [('==', 'c:/test.checkout/LayoutTests/passes/svgreftest-expected.svg')] != [('==', '/test.checkout/LayoutTests/passes/svgreftest-expected.svg')]
> 
> ----------------------------------------------------------------------

:-(  i knew it. will fix ASAP.
Comment 13 Ryosuke Niwa 2012-02-03 19:52:31 PST
Fix landed in http://trac.webkit.org/changeset/106726.