Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2098843002:
Invalidate a WebKit test which is testing for the wrong semantics (Closed)
DescriptionInvalidate a WebKit test which is testing for the wrong semantics in
TypedArray constructors. Fixing the semantics in V8 broke the test.
See crrev.com/3f0ada1d84e8956213465cb30b48e66f83895e8f
BUG=623120
Committed: https://crrev.com/b57d8f8d9e7c1a26f94c4715804c9457e27cd5cf
Cr-Commit-Position: refs/heads/master@{#402511}
Patch Set 1 #
Total comments: 2
Patch Set 2 : update webkit test expections for crbug.com/623120 #Messages
Total messages: 26 (12 generated)
Description was changed from ========== Invalidates a WebKit test which is testing for the wrong semantics in ArrayBuffer constructors. Fixing the semantics in V8 broke the test. BUG=623120 ========== to ========== Invalidate a WebKit test which is testing for the wrong semantics in ArrayBuffer constructors. Fixing the semantics in V8 broke the test. BUG=623120 ==========
bakkot@google.com changed reviewers: + littledan@chromium.org
Description was changed from ========== Invalidate a WebKit test which is testing for the wrong semantics in ArrayBuffer constructors. Fixing the semantics in V8 broke the test. BUG=623120 ========== to ========== Invalidate a WebKit test which is testing for the wrong semantics in ArrayBuffer constructors. Fixing the semantics in V8 broke the test. See crrev.com/3f0ada1d84e8956213465cb30b48e66f83895e8f BUG=623120 ==========
The CQ bit was checked by littledan@chromium.org
lgtm
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
No L-G-T-M from a valid reviewer yet. CQ run can only be started by full committers or once the patch has received an L-G-T-M from a full committer. Even if an L-G-T-M may have been provided, it was from a non-committer, _not_ a full super star committer. See http://www.chromium.org/getting-involved/become-a-committer Note that this has nothing to do with OWNERS files.
bakkot@google.com changed reviewers: + adamk@chromium.org
ptal
Is the new behavior more or less permissive? https://codereview.chromium.org/2098843002/diff/1/third_party/WebKit/LayoutTe... File third_party/WebKit/LayoutTests/TestExpectations (right): https://codereview.chromium.org/2098843002/diff/1/third_party/WebKit/LayoutTe... third_party/WebKit/LayoutTests/TestExpectations:876: # Webkit test is checking for outdated semantics on ArrayBuffer constructors. Do you mean "TypedArray" here? The test seems like it's asserting things about various kinds of TypedArrays, not the underlying ArrayBuffers. If I'm right, please fix the CL description too.
Description was changed from ========== Invalidate a WebKit test which is testing for the wrong semantics in ArrayBuffer constructors. Fixing the semantics in V8 broke the test. See crrev.com/3f0ada1d84e8956213465cb30b48e66f83895e8f BUG=623120 ========== to ========== Invalidate a WebKit test which is testing for the wrong semantics in TypedArray constructors. Fixing the semantics in V8 broke the test. See crrev.com/3f0ada1d84e8956213465cb30b48e66f83895e8f BUG=623120 ==========
On 2016/06/27 at 19:12:08, adamk wrote: > Is the new behavior more or less permissive? More. Formerly calling e.g. new Int8Array(null) would throw; now it returns an array of length 0. > https://codereview.chromium.org/2098843002/diff/1/third_party/WebKit/LayoutTe... > File third_party/WebKit/LayoutTests/TestExpectations (right): > > https://codereview.chromium.org/2098843002/diff/1/third_party/WebKit/LayoutTe... > third_party/WebKit/LayoutTests/TestExpectations:876: # Webkit test is checking for outdated semantics on ArrayBuffer constructors. > Do you mean "TypedArray" here? The test seems like it's asserting things about various kinds of TypedArrays, not the underlying ArrayBuffers. > > If I'm right, please fix the CL description too. Quite right, done.
https://codereview.chromium.org/2098843002/diff/1/third_party/WebKit/LayoutTe... File third_party/WebKit/LayoutTests/TestExpectations (right): https://codereview.chromium.org/2098843002/diff/1/third_party/WebKit/LayoutTe... third_party/WebKit/LayoutTests/TestExpectations:876: # Webkit test is checking for outdated semantics on ArrayBuffer constructors. On 2016/06/27 at 19:12:08, adamk wrote: > Do you mean "TypedArray" here? The test seems like it's asserting things about various kinds of TypedArrays, not the underlying ArrayBuffers. > > If I'm right, please fix the CL description too. Done.
lgtm
The CQ bit was checked by bakkot@google.com 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: This issue passed the CQ dry run.
The CQ bit was checked by bakkot@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from littledan@chromium.org Link to the patchset: https://codereview.chromium.org/2098843002/#ps20001 (title: "update webkit test expections for crbug.com/623120")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Invalidate a WebKit test which is testing for the wrong semantics in TypedArray constructors. Fixing the semantics in V8 broke the test. See crrev.com/3f0ada1d84e8956213465cb30b48e66f83895e8f BUG=623120 ========== to ========== Invalidate a WebKit test which is testing for the wrong semantics in TypedArray constructors. Fixing the semantics in V8 broke the test. See crrev.com/3f0ada1d84e8956213465cb30b48e66f83895e8f BUG=623120 Committed: https://crrev.com/b57d8f8d9e7c1a26f94c4715804c9457e27cd5cf Cr-Commit-Position: refs/heads/master@{#402511} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/b57d8f8d9e7c1a26f94c4715804c9457e27cd5cf Cr-Commit-Position: refs/heads/master@{#402511} |
