Bug 124714 – Instead of a large 'if' block, each failure class should write its own result in test_result_writer.py

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Dániel Bátyai]

# FIXME: Instead of this long 'if' block, each failure class might
# have a responsibility for writing a test result.

[Dániel Bátyai]

Created attachment 217562 [details]
Proposed patch

[WebKit Commit Bot]

Attachment 217562 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Tools/ChangeLog', u'Tools/Scripts/webkitpy/layout_tests/controllers/test_result_writer.py', u'Tools/Scripts/webkitpy/layout_tests/models/test_failures.py']" exit_code: 1
Tools/Scripts/webkitpy/layout_tests/models/test_failures.py:214:  [FailureReftestMismatch.write_result] Undefined variable '_log'  [pylint/E0602] [5]
Total errors found: 1 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Dániel Bátyai]

Created attachment 217564 [details]
Proposed patch

[Ryosuke Niwa]

Comment on attachment 217564 [details]
Proposed patch

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

> Tools/Scripts/webkitpy/layout_tests/models/test_failures.py:153
>  class FailureMissingResult(TestFailure):
>      def message(self):
>          return "-expected.txt was missing"
>  
> +    def write_failure(self, writer, driver_output, expected_driver_output, port):
> +        writer.write_text_files(driver_output.text, expected_driver_output.text)
> +        writer.create_text_diff_and_write_result(driver_output.text, expected_driver_output.text)

Can we create a common superclass of FailureMissingResult and FailureTextMismatch to share the code here?

> Tools/Scripts/webkitpy/layout_tests/models/test_failures.py:247
> +    def write_failure(self, writer, driver_output, expected_driver_output, port):
> +        writer.write_audio_files(driver_output.audio, expected_driver_output.audio)
> +

Ditto about extracting a common superclass for FailureMissingAudio and FailureAudioMismatch.

[Dániel Bátyai]

Created attachment 217665 [details]
Proposed patch

[WebKit Commit Bot]

Attachment 217665 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Tools/ChangeLog', u'Tools/Scripts/webkitpy/layout_tests/controllers/test_result_writer.py', u'Tools/Scripts/webkitpy/layout_tests/models/test_failures.py']" exit_code: 1
Tools/Scripts/webkitpy/layout_tests/models/test_failures.py:115:  expected 2 blank lines, found 1  [pep8/E302] [5]
Tools/Scripts/webkitpy/layout_tests/models/test_failures.py:120:  expected 2 blank lines, found 1  [pep8/E302] [5]
Tools/Scripts/webkitpy/layout_tests/models/test_failures.py:155:  expected 2 blank lines, found 1  [pep8/E302] [5]
Tools/Scripts/webkitpy/layout_tests/models/test_failures.py:159:  expected 2 blank lines, found 1  [pep8/E302] [5]
Total errors found: 4 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Dániel Bátyai]

Created attachment 217666 [details]
Proposed patch

[Dániel Bátyai]

Can I get a review on this please?

[WebKit Commit Bot]

Comment on attachment 217666 [details]
Proposed patch

Clearing flags on attachment: 217666

Committed r159978: <http://trac.webkit.org/changeset/159978>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.