Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2597723002:
Convert audiobuffer-resample to testharness (Closed)
|
Created:
4 years ago by Raymond Toy Modified:
3 years, 11 months ago Reviewers:
hongchan CC:
chromium-reviews, blink-reviews Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionConvert audiobuffer-resample to testharness
Manually convert test to use testharness
BUG=676355
TEST=audiobuffer-resample.html
Review-Url: https://codereview.chromium.org/2597723002
Cr-Commit-Position: refs/heads/master@{#441686}
Committed: https://chromium.googlesource.com/chromium/src/+/234116bf576be1dc6e43232753d3065849e6e816
Patch Set 1 #Patch Set 2 : Rebase and remove unused expected results #
Total comments: 8
Patch Set 3 : Address review comments. #
Messages
Total messages: 19 (12 generated)
The CQ bit was checked by rtoy@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
Description was changed from ========== Convert AudioBuffer tests to testharness Manually convert test to use testharness BUG=676355 TEST=audiobuffer-resample.html ========== to ========== Convert audiobuffer-resample to testharness Manually convert test to use testharness BUG=676355 TEST=audiobuffer-resample.html ==========
rtoy@chromium.org changed reviewers: + hongchan@chromium.org
PTAL
lgtm with nits https://codereview.chromium.org/2597723002/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/webaudio/AudioBuffer/audiobuffer-resample.html (right): https://codereview.chromium.org/2597723002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/webaudio/AudioBuffer/audiobuffer-resample.html:1: <!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"> If this file includes manual changes, please simplify this line like the other layout test. We don't need all these meaningless texts in the tag. https://codereview.chromium.org/2597723002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/webaudio/AudioBuffer/audiobuffer-resample.html:85: .beLessThan(peakThreshold); It looks like the new audit pays off. Happy to have this change. https://codereview.chromium.org/2597723002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/webaudio/AudioBuffer/audiobuffer-resample.html:91: audit.define("Test resampling of an AudioBuffer", function (task, should) { Please wrap this line. https://codereview.chromium.org/2597723002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/webaudio/AudioBuffer/audiobuffer-resample.html:95: // Create a sine wave in a buffer that's different from the context rate to test ditto.
https://codereview.chromium.org/2597723002/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/webaudio/AudioBuffer/audiobuffer-resample.html (right): https://codereview.chromium.org/2597723002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/webaudio/AudioBuffer/audiobuffer-resample.html:1: <!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"> On 2017/01/04 00:38:12, hongchan wrote: > If this file includes manual changes, please simplify this line like the other > layout test. We don't need all these meaningless texts in the tag. Done. https://codereview.chromium.org/2597723002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/webaudio/AudioBuffer/audiobuffer-resample.html:85: .beLessThan(peakThreshold); On 2017/01/04 00:38:12, hongchan wrote: > It looks like the new audit pays off. Happy to have this change. To be fair, the old Audit would have simplified this as well. :-) https://codereview.chromium.org/2597723002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/webaudio/AudioBuffer/audiobuffer-resample.html:91: audit.define("Test resampling of an AudioBuffer", function (task, should) { On 2017/01/04 00:38:12, hongchan wrote: > Please wrap this line. Done. https://codereview.chromium.org/2597723002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/webaudio/AudioBuffer/audiobuffer-resample.html:95: // Create a sine wave in a buffer that's different from the context rate to test On 2017/01/04 00:38:12, hongchan wrote: > ditto. Done.
The CQ bit was checked by rtoy@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from hongchan@chromium.org Link to the patchset: https://codereview.chromium.org/2597723002/#ps40001 (title: "Address review comments.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by rtoy@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1483635249359910,
"parent_rev": "3b07e290a4a4273bc516a5d5f8184793075b7f84", "commit_rev":
"234116bf576be1dc6e43232753d3065849e6e816"}
Message was sent while issue was closed.
Description was changed from ========== Convert audiobuffer-resample to testharness Manually convert test to use testharness BUG=676355 TEST=audiobuffer-resample.html ========== to ========== Convert audiobuffer-resample to testharness Manually convert test to use testharness BUG=676355 TEST=audiobuffer-resample.html Review-Url: https://codereview.chromium.org/2597723002 Cr-Commit-Position: refs/heads/master@{#441686} Committed: https://chromium.googlesource.com/chromium/src/+/234116bf576be1dc6e43232753d3... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/234116bf576be1dc6e43232753d3... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
