Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2839913002:
[modules] Add simple test concerning instantiation failure. (Closed)
|
Created:
3 years, 8 months ago by neis Modified:
3 years, 7 months ago CC:
blink-reviews, blink-reviews-w3ctests_chromium.org, chromium-reviews, David Black, donnd+watch_chromium.org, Jered, jfweitz+watch_chromium.org, kmadhusu+watch_chromium.org, melevin+watch_chromium.org, module-dev_chromium.org, samarth+watch_chromium.org, skanuj+watch_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
Description[modules] Add simple test concerning instantiation failure.
BUG=chromium:714521
Review-Url: https://codereview.chromium.org/2839913002
Cr-Commit-Position: refs/heads/master@{#467669}
Committed: https://chromium.googlesource.com/chromium/src/+/f6089442785c7dd2e1fcfa54f53e38e3f7b951f6
Patch Set 1 #Patch Set 2 : Allow uncaught exception. #Patch Set 3 : Add comment. #
Messages
Total messages: 27 (14 generated)
Description was changed from ========== [modules] Add test concerning instantiation failure. BUG= ========== to ========== [modules] Add simple test concerning instantiation failure. BUG= ==========
neis@chromium.org changed reviewers: + domenic@chromium.org
lgtm
On 2017/04/25 18:54:15, domenic wrote: > lgtm Would you add bug # 714521?
hiroshige@chromium.org changed reviewers: + hiroshige@chromium.org
What is the expecation of this test? No crash? Also comments that describes what this is testing (e.g. corresponing spec conditions) is helpful. (I'm not familiar with wpt test styles though)
On 2017/04/25 20:37:39, hiroshige wrote: > What is the expecation of this test? No crash? Yeah, for now. When the error reporting semantics is clear, I will update the test. > Also comments that describes what this is testing (e.g. corresponing spec > conditions) is helpful. > (I'm not familiar with wpt test styles though) Neither am I, so I'll leave it up to Domenic. I think at the moment the test is pretty self-explanatory.
Description was changed from ========== [modules] Add simple test concerning instantiation failure. BUG= ========== to ========== [modules] Add simple test concerning instantiation failure. BUG=chromium:714521 ==========
On 2017/04/26 at 08:31:17, neis wrote: > On 2017/04/25 20:37:39, hiroshige wrote: > > What is the expecation of this test? No crash? > > Yeah, for now. When the error reporting semantics is clear, I will update the test. > > > Also comments that describes what this is testing (e.g. corresponing spec > > conditions) is helpful. > > (I'm not familiar with wpt test styles though) > > Neither am I, so I'll leave it up to Domenic. I think at the moment the test is pretty self-explanatory. I guess it is a good point that it is hard to know what this is testing. I'd add a HTML comment like: <!-- This test case caught a crash bug in Chrome, due to a logic bug in the spec when a second <script> element tried to load a previously-failed module. As such, the test passes as long as nothing crashes. See https://github.com/whatwg/html/pull/2559 for background. -->
The CQ bit was checked by kouhei@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: 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 neis@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from domenic@chromium.org Link to the patchset: https://codereview.chromium.org/2839913002/#ps40001 (title: "Add comment.")
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 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. Committers are members of the group "project-chromium-committers". Note that this has nothing to do with OWNERS files.
lgtm % TestExpectations file
On 2017/04/27 12:49:49, kouhei wrote: > lgtm % TestExpectations file Sorry, no change in TestExpectations file needed. I overlooked PS2 trybots.
The CQ bit was checked by neis@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": 1493299047648530,
"parent_rev": "a347f0ebcb36d711a0ecb366b15815ce595f64bd", "commit_rev":
"f6089442785c7dd2e1fcfa54f53e38e3f7b951f6"}
Message was sent while issue was closed.
Description was changed from ========== [modules] Add simple test concerning instantiation failure. BUG=chromium:714521 ========== to ========== [modules] Add simple test concerning instantiation failure. BUG=chromium:714521 Review-Url: https://codereview.chromium.org/2839913002 Cr-Commit-Position: refs/heads/master@{#467669} Committed: https://chromium.googlesource.com/chromium/src/+/f6089442785c7dd2e1fcfa54f53e... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/f6089442785c7dd2e1fcfa54f53e...
Message was sent while issue was closed.
lgtm |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
