Bug 77685 – Ref Tests should support plain SVG files

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Dirk Schulze]

Ref Tests should support plain SVG files with the ending .svg. It would be great to have them for SVG tests as well.

[Hayato Ito]

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?

[Ryosuke Niwa]

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

[Ryosuke Niwa]

A patch coming...

[Ryosuke Niwa]

Created attachment 125301 [details]
fixes the bug

[Tony Chang]

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?

[Ryosuke Niwa]

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.

[Ryosuke Niwa]

I'm gonna just get rid of is_reftest. It's a useless function now.

[Ryosuke Niwa]

Created attachment 125379 [details]
fixes the bug

[Ryosuke Niwa]

Comment on attachment 125379 [details]
fixes the bug

Clearing flags on attachment: 125379

Committed r106692: <http://trac.webkit.org/changeset/106692>

[Ryosuke Niwa]

All reviewed patches have been landed.  Closing bug.

[James Robinson]

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')]

----------------------------------------------------------------------

[Ryosuke Niwa]

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

[Ryosuke Niwa]

Fix landed in http://trac.webkit.org/changeset/106726.