|
Description
Hyemi Shin
2015-10-21 00:03:24 PDT
Created attachment 263664 [details]
Patch
Comment on attachment 263664 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=263664&action=review > Source/WebCore/ChangeLog:11 > + No new tests. Why? We need a test demonstrating this correct behavior. Created attachment 263769 [details]
Patch
I'm not sure I made new test correctly.. Please take a look. Comment on attachment 263769 [details] Patch Attachment 263769 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/318438 New failing tests: webaudio/convolver-setBuffer-different-samplerate.html Created attachment 263771 [details]
Archive of layout-test-results from ews107 for mac-mavericks-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
Comment on attachment 263769 [details] Patch Attachment 263769 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/318564 New failing tests: webaudio/convolver-setBuffer-different-samplerate.html Created attachment 263773 [details]
Archive of layout-test-results from ews100 for mac-mavericks
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100 Port: mac-mavericks Platform: Mac OS X 10.9.5
Comment on attachment 263769 [details] Patch Attachment 263769 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/318523 New failing tests: webaudio/convolver-setBuffer-different-samplerate.html Created attachment 263775 [details]
Archive of layout-test-results from ews112 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112 Port: mac-yosemite Platform: Mac OS X 10.10.5
Created attachment 263779 [details]
Patch
Comment on attachment 263779 [details] Patch Attachment 263779 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/319280 New failing tests: webaudio/convolver-setBuffer-different-samplerate.html Created attachment 263783 [details]
Archive of layout-test-results from ews101 for mac-mavericks
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101 Port: mac-mavericks Platform: Mac OS X 10.9.5
Comment on attachment 263779 [details] Patch Attachment 263779 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/319292 New failing tests: webaudio/convolver-setBuffer-different-samplerate.html Created attachment 263784 [details]
Archive of layout-test-results from ews104 for mac-mavericks-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
Could anyone help me to fix test failiures on mavericks and yosemite only? Comment on attachment 263779 [details] Patch Attachment 263779 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/319293 New failing tests: webaudio/convolver-setBuffer-different-samplerate.html Created attachment 263785 [details]
Archive of layout-test-results from ews115 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115 Port: mac-yosemite Platform: Mac OS X 10.10.5
Created attachment 263792 [details]
Patch
Comment on attachment 263792 [details] Patch Attachment 263792 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/319993 New failing tests: webaudio/convolver-setBuffer-different-samplerate.html Created attachment 263794 [details]
Archive of layout-test-results from ews103 for mac-mavericks
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103 Port: mac-mavericks Platform: Mac OS X 10.9.5
Created attachment 263795 [details]
Patch
Comment on attachment 263795 [details] Patch Attachment 263795 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/320134 New failing tests: webaudio/convolver-setBuffer-different-samplerate.html Created attachment 263796 [details]
Archive of layout-test-results from ews100 for mac-mavericks
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100 Port: mac-mavericks Platform: Mac OS X 10.9.5
Created attachment 263797 [details]
Patch
(In reply to comment #25) > Created attachment 263797 [details] > Patch r=me. Thanks! (In reply to comment #26) > (In reply to comment #25) > > Created attachment 263797 [details] > > Patch > > r=me. Thanks! Thanks! could you give me cq+? Comment on attachment 263797 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=263797&action=review > LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:1 > +<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"> <!DOCTYPE html> > LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:9 > +<div id="description"></div> Not needed. > LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:10 > +<div id="console"></div> Not needed. > LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:15 > +function runTest() You don't really need a function. > LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:17 > + if (window.testRunner) { This whole if block is not needed. > LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:22 > + window.jsTestIsAsync = true; I don't see why this test needs to be async. > LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:25 > + var conv = context.createConvolver(); No abbreviations in variable names please. > LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:27 > + try { shouldThrow("conv.buffer = context.createBuffer(1, 256, 22050)", "'Error: NOT_SUPPORTED_ERR'"); (or similar). > LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:37 > + try { Could use shouldThrow() as well. > LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:44 > + finishJSTest(); Not needed. > LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:48 > +successfullyParsed = true; Should not be needed. Comment on attachment 263797 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=263797&action=review > Source/WebCore/ChangeLog:11 > + Test added to LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html (amended) Test: webaudio/convolver-setBuffer-different-samplerate.html (In reply to comment #28) > Comment on attachment 263797 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=263797&action=review > > > LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:27 > > + try { > > shouldThrow("conv.buffer = context.createBuffer(1, 256, 22050)", "'Error: > NOT_SUPPORTED_ERR'"); (or similar). > > > LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:37 > > + try { > > Could use shouldThrow() I used shouldThrow() and shouldNotThrow() at first, but it made an error on Mavericks and Yosemite. That's because why I use try-catch. Rest of comments will be applied in next patch attachment. Thanks. (In reply to comment #30) > (In reply to comment #28) > > Comment on attachment 263797 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=263797&action=review > > > > > LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:27 > > > + try { > > > > shouldThrow("conv.buffer = context.createBuffer(1, 256, 22050)", "'Error: > > NOT_SUPPORTED_ERR'"); (or similar). > > > > > LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:37 > > > + try { > > > > Could use shouldThrow() > > I used shouldThrow() and shouldNotThrow() at first, but it made an error on > Mavericks and Yosemite. That's because why I use try-catch. Rest of comments > will be applied in next patch attachment. Thanks. There is no reason for shouldThrow() to behave different on one platform vs another. (In reply to comment #31) > (In reply to comment #30) > > (In reply to comment #28) > > > Comment on attachment 263797 [details] > > > Patch > > > > > > View in context: > > > https://bugs.webkit.org/attachment.cgi?id=263797&action=review > > > > > > > LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:27 > > > > + try { > > > > > > shouldThrow("conv.buffer = context.createBuffer(1, 256, 22050)", "'Error: > > > NOT_SUPPORTED_ERR'"); (or similar). > > > > > > > LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:37 > > > > + try { > > > > > > Could use shouldThrow() > > > > I used shouldThrow() and shouldNotThrow() at first, but it made an error on > > Mavericks and Yosemite. That's because why I use try-catch. Rest of comments > > will be applied in next patch attachment. Thanks. > > There is no reason for shouldThrow() to behave different on one platform vs > another. I looked at your previous diffs and saw the following error: +PASS conv.buffer = context.createBuffer(1, 100, 22050) threw exception ReferenceError: Can't find variable: conv. The issue was that the 'conv' variable was in the runTest() function scope, which is not compatible with should*() functions. Once you get rid of the runTest() function as I suggested, you WILL be able to use shouldThrow() again (please do). Note that alternatively, you could have declared 'conv' at global level by omitting the 'var'. (In reply to comment #32) > (In reply to comment #31) > > (In reply to comment #30) > > > (In reply to comment #28) > > > > Comment on attachment 263797 [details] > > > > Patch > > > > > > > > View in context: > > > > https://bugs.webkit.org/attachment.cgi?id=263797&action=review > > > > > > > > > LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:27 > > > > > + try { > > > > > > > > shouldThrow("conv.buffer = context.createBuffer(1, 256, 22050)", "'Error: > > > > NOT_SUPPORTED_ERR'"); (or similar). > > > > > > > > > LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:37 > > > > > + try { > > > > > > > > Could use shouldThrow() > > > > > > I used shouldThrow() and shouldNotThrow() at first, but it made an error on > > > Mavericks and Yosemite. That's because why I use try-catch. Rest of comments > > > will be applied in next patch attachment. Thanks. > > > > There is no reason for shouldThrow() to behave different on one platform vs > > another. > > I looked at your previous diffs and saw the following error: > +PASS conv.buffer = context.createBuffer(1, 100, 22050) threw exception > ReferenceError: Can't find variable: conv. > > The issue was that the 'conv' variable was in the runTest() function scope, > which is not compatible with should*() functions. Once you get rid of the > runTest() function as I suggested, you WILL be able to use shouldThrow() > again (please do). > > Note that alternatively, you could have declared 'conv' at global level by > omitting the 'var'. Ok, i will try. Thanks a lot. Created attachment 263890 [details]
Patch
Comment on attachment 263890 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=263890&action=review > LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:15 > +shouldThrow("convolver.buffer = context.createBuffer(1, 256, 22050)"); Please specify the exception as second parameter: shouldThrow("convolver.buffer = context.createBuffer(1, 256, 22050)", "'Error: NotSupportedError: DOM Exception 9'"); > LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:18 > +runTest(); Forgot to remove this. Comment on attachment 263890 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=263890&action=review > Source/WebCore/ChangeLog:11 > + Test added to LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html Also, this should look like: Test: webaudio/convolver-setBuffer-different-samplerate.html Comment on attachment 263890 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=263890&action=review > LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:1 > +<!DOCTYPE HTML"> again, this should be: <!DOCTYPE html> Created attachment 263893 [details]
Patch
Comment on attachment 263893 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=263893&action=review This will NOT pass EWS. > Source/WebCore/ChangeLog:11 > + Test : webaudio/convolver-setBuffer-different-samplerate.html Test: > LayoutTests/webaudio/convolver-setBuffer-different-samplerate-expected.txt:5 > +PASS conv.buffer = context.createBuffer(1, 256, 22050) threw exception Error: NotSupportedError: DOM Exception 9. conv -> convolver > LayoutTests/webaudio/convolver-setBuffer-different-samplerate-expected.txt:6 > +PASS conv.buffer = context.createBuffer(1, 256, 44100) did not throw exception ditto. > LayoutTests/webaudio/convolver-setBuffer-different-samplerate.html:16 > +shouldNotThrow("conv.buffer = context.createBuffer(1, 256, 44100)"); Please run your test.. conv -> convolver Created attachment 263895 [details]
Patch
Comment on attachment 263895 [details] Patch Attachment 263895 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/325579 New failing tests: webaudio/convolver-setBuffer-different-samplerate.html Created attachment 263897 [details]
Archive of layout-test-results from ews101 for mac-mavericks
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101 Port: mac-mavericks Platform: Mac OS X 10.9.5
How do you generate your test expectations? The diff looks like so: http://pastebin.com/dn0d7FD5 (In reply to comment #43) > How do you generate your test expectations? > > The diff looks like so: > http://pastebin.com/dn0d7FD5 We usually do: Tools/Scripts/run-webkit-tests --reset-results webaudio/convolver-setBuffer-different-samplerate.html Comment on attachment 263895 [details] Patch Attachment 263895 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/325589 New failing tests: webaudio/convolver-setBuffer-different-samplerate.html Created attachment 263898 [details]
Archive of layout-test-results from ews107 for mac-mavericks-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
Comment on attachment 263895 [details] Patch Attachment 263895 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/325629 New failing tests: webaudio/convolver-setBuffer-different-samplerate.html Created attachment 263901 [details]
Archive of layout-test-results from ews112 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112 Port: mac-yosemite Platform: Mac OS X 10.10.5
Created attachment 263905 [details]
Patch
Comment on attachment 263905 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=263905&action=review r=me > Source/WebCore/ChangeLog:11 > + Test : webaudio/convolver-setBuffer-different-samplerate.html In the future, please no space before the ':'. No need to reupload for this though. (In reply to comment #50) > Comment on attachment 263905 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=263905&action=review > > r=me > > > Source/WebCore/ChangeLog:11 > > + Test : webaudio/convolver-setBuffer-different-samplerate.html > > In the future, please no space before the ':'. No need to reupload for this > though. Thank you for your advices! Comment on attachment 263905 [details] Patch Clearing flags on attachment: 263905 Committed r191492: <http://trac.webkit.org/changeset/191492> All reviewed patches have been landed. Closing bug. |