[webkit-reviews] review denied: [Bug 57987] new-run-webkit-tests: implement support for audio files : [Attachment 88524] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Apr 6 16:04:59 PDT 2011
Tony Chang <tony at chromium.org> has denied Dirk Pranke <dpranke at chromium.org>'s
request for review:
Bug 57987: new-run-webkit-tests: implement support for audio files
https://bugs.webkit.org/show_bug.cgi?id=57987
Attachment 88524: Patch
https://bugs.webkit.org/attachment.cgi?id=88524&action=review
------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=88524&action=review
r- because no tests and until it's clear whether we have .txt and .wav or just
.wav.
> Tools/Scripts/webkitpy/layout_tests/layout_package/single_test_runner.py:85
> + for suffix in ['.txt', '.checksum', '.png', '.wav']:
Nit: [] to ()
> Tools/Scripts/webkitpy/layout_tests/layout_package/single_test_runner.py:241
> + if expected_audio == '':
'' or None? It looks like this will be None if the file is not found. Maybe
just change this to 'if not expected_audio:'
> Tools/Scripts/webkitpy/layout_tests/layout_package/test_expectations.py:44
> +(PASS, FAIL, TEXT, IMAGE, IMAGE_PLUS_TEXT, AUDIO, AUDIO_PLUS_TEXT, TIMEOUT,
I thought these tests only generated .wav files (i.e., no text file)?
> Tools/Scripts/webkitpy/layout_tests/port/webkit.py:447
> elif line.startswith('Content-Type:'):
> - pass
> + content_type = ' '.split(line)[1]
> + elif line.startswith('Content-Transfer-Encoding:'):
Should Content-Type: and Content-Transfer-Encoding: be consts like
Content-Length:?
More information about the webkit-reviews
mailing list