Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1568433004:
[promise] unskip more passing Test262 tests (Closed)
|
Created:
4 years, 11 months ago by caitp (gmail) Modified:
4 years, 11 months ago CC:
Sven Panne, v8-reviews_googlegroups.com Base URL:
https://chromium.googlesource.com/v8/v8.git@master Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
Description[promise] unskip more passing Test262 tests
BUG=v8:4633
LOG=N
R=adamk@chromium.org, littledan@chromium.org, cbruni@chromium.org
Committed: https://crrev.com/c12a47a52059dc9464b14d2276f176491eebd9cf
Cr-Commit-Position: refs/heads/master@{#33160}
Patch Set 1 #Patch Set 2 : Skip test in ignition #Patch Set 3 : Skip in asan builds #
Total comments: 1
Patch Set 4 : Rebase #Patch Set 5 : Add a bug reference #Messages
Total messages: 29 (9 generated)
Description was changed from ========== [promise] unskip more passing Test262 test BUG=v8:4633 LOG=N R=adamk@chromium.org, littledan@chromium.org, cbruni@chromium.org ========== to ========== [promise] unskip more passing Test262 tests BUG=v8:4633 LOG=N R=adamk@chromium.org, littledan@chromium.org, cbruni@chromium.org ==========
Quick test enabling CL
The CQ bit was checked by littledan@chromium.org
lgtm
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1568433004/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1568433004/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: v8_linux64_asan_rel on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel/builds/...)
On 2016/01/06 17:39:10, commit-bot: I haz the power wrote: > Try jobs failed on following builders: > v8_linux64_asan_rel on tryserver.v8 (JOB_FAILED, > http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel/builds/...) hmmm --- --omit-quit seems to break $DONE in the test262 harness. I'm not sure how that wouldn't have broken things before though
On 2016/01/06 at 17:44:19, caitpotter88 wrote: > On 2016/01/06 17:39:10, commit-bot: I haz the power wrote: > > Try jobs failed on following builders: > > v8_linux64_asan_rel on tryserver.v8 (JOB_FAILED, > > http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel/builds/...) > > hmmm --- --omit-quit seems to break $DONE in the test262 harness. I'm not sure how that wouldn't have broken things before though What I would do is mark the right tests as PASS so the precq gets further, all the way to ignition, and then see what happens in the trybot.
The CQ bit was checked by littledan@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1568433004/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1568433004/20001
On 2016/01/06 at 17:46:50, Dan Ehrenberg wrote: > On 2016/01/06 at 17:44:19, caitpotter88 wrote: > > On 2016/01/06 17:39:10, commit-bot: I haz the power wrote: > > > Try jobs failed on following builders: > > > v8_linux64_asan_rel on tryserver.v8 (JOB_FAILED, > > > http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel/builds/...) > > > > hmmm --- --omit-quit seems to break $DONE in the test262 harness. I'm not sure how that wouldn't have broken things before though > > What I would do is mark the right tests as PASS so the precq gets further, all the way to ignition, and then see what happens in the trybot. Oh sorry wrong thread!
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_linux64_asan_rel on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel/builds/...)
On 2016/01/06 17:47:50, Dan Ehrenberg wrote: > On 2016/01/06 at 17:46:50, Dan Ehrenberg wrote: > > On 2016/01/06 at 17:44:19, caitpotter88 wrote: > > > On 2016/01/06 17:39:10, commit-bot: I haz the power wrote: > > > > Try jobs failed on following builders: > > > > v8_linux64_asan_rel on tryserver.v8 (JOB_FAILED, > > > > > http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel/builds/...) > > > > > > hmmm --- --omit-quit seems to break $DONE in the test262 harness. I'm not > sure how that wouldn't have broken things before though > > > > What I would do is mark the right tests as PASS so the precq gets further, all > the way to ignition, and then see what happens in the trybot. > > Oh sorry wrong thread! The comment in d8.cc looks like --omit-quit is mainly for emscripten's benefit. Maybe test262 runs on the asan bot don't need this flag. Maybe you are able to look at the v8_linux64_asan_rel recipe and update the test262 flags?
On 2016/01/06 18:15:49, caitp wrote: > On 2016/01/06 17:47:50, Dan Ehrenberg wrote: > > On 2016/01/06 at 17:46:50, Dan Ehrenberg wrote: > > > On 2016/01/06 at 17:44:19, caitpotter88 wrote: > > > > On 2016/01/06 17:39:10, commit-bot: I haz the power wrote: > > > > > Try jobs failed on following builders: > > > > > v8_linux64_asan_rel on tryserver.v8 (JOB_FAILED, > > > > > > > > http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel/builds/...) > > > > > > > > hmmm --- --omit-quit seems to break $DONE in the test262 harness. I'm not > > sure how that wouldn't have broken things before though > > > > > > What I would do is mark the right tests as PASS so the precq gets further, > all > > the way to ignition, and then see what happens in the trybot. > > > > Oh sorry wrong thread! > > The comment in d8.cc looks like --omit-quit is mainly for emscripten's benefit. > Maybe test262 runs on the asan bot don't need this flag. > > Maybe you are able to look at the v8_linux64_asan_rel recipe and update the > test262 flags? +svennepanne --- 9bb8b58549d543bacc230b4094ff5c196fa61ad3 adds --omit-quit, but the test262 harness makes use of this function, especially to terminate asynchronous tests. What other options do we have to actually run the affected tests?
On 2016/01/06 18:44:38, caitp wrote: > On 2016/01/06 18:15:49, caitp wrote: > > On 2016/01/06 17:47:50, Dan Ehrenberg wrote: > > > On 2016/01/06 at 17:46:50, Dan Ehrenberg wrote: > > > > On 2016/01/06 at 17:44:19, caitpotter88 wrote: > > > > > On 2016/01/06 17:39:10, commit-bot: I haz the power wrote: > > > > > > Try jobs failed on following builders: > > > > > > v8_linux64_asan_rel on tryserver.v8 (JOB_FAILED, > > > > > > > > > > > > http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel/builds/...) > > > > > > > > > > hmmm --- --omit-quit seems to break $DONE in the test262 harness. I'm > not > > > sure how that wouldn't have broken things before though > > > > > > > > What I would do is mark the right tests as PASS so the precq gets further, > > all > > > the way to ignition, and then see what happens in the trybot. > > > > > > Oh sorry wrong thread! > > > > The comment in d8.cc looks like --omit-quit is mainly for emscripten's > benefit. > > Maybe test262 runs on the asan bot don't need this flag. > > > > Maybe you are able to look at the v8_linux64_asan_rel recipe and update the > > test262 flags? > > +svennepanne --- 9bb8b58549d543bacc230b4094ff5c196fa61ad3 adds --omit-quit, but > the test262 harness makes use of this function, especially to terminate > asynchronous tests. What other options do we have to actually run the affected > tests? Sven no longer works on V8. It seems like we should be able to narrow the circumstances under which this flag is needed.
On 2016/01/06 19:04:15, adamk wrote: > On 2016/01/06 18:44:38, caitp wrote: > > On 2016/01/06 18:15:49, caitp wrote: > > > On 2016/01/06 17:47:50, Dan Ehrenberg wrote: > > > > On 2016/01/06 at 17:46:50, Dan Ehrenberg wrote: > > > > > On 2016/01/06 at 17:44:19, caitpotter88 wrote: > > > > > > On 2016/01/06 17:39:10, commit-bot: I haz the power wrote: > > > > > > > Try jobs failed on following builders: > > > > > > > v8_linux64_asan_rel on tryserver.v8 (JOB_FAILED, > > > > > > > > > > > > > > > > > http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel/builds/...) > > > > > > > > > > > > hmmm --- --omit-quit seems to break $DONE in the test262 harness. I'm > > not > > > > sure how that wouldn't have broken things before though > > > > > > > > > > What I would do is mark the right tests as PASS so the precq gets > further, > > > all > > > > the way to ignition, and then see what happens in the trybot. > > > > > > > > Oh sorry wrong thread! > > > > > > The comment in d8.cc looks like --omit-quit is mainly for emscripten's > > benefit. > > > Maybe test262 runs on the asan bot don't need this flag. > > > > > > Maybe you are able to look at the v8_linux64_asan_rel recipe and update the > > > test262 flags? > > > > +svennepanne --- 9bb8b58549d543bacc230b4094ff5c196fa61ad3 adds --omit-quit, > but > > the test262 harness makes use of this function, especially to terminate > > asynchronous tests. What other options do we have to actually run the affected > > tests? > > Sven no longer works on V8. It seems like we should be able to narrow the > circumstances under which this flag is needed. E.g., you could add code to test/mjsunit/testcfg.py to add --omit-quit if the path contains "/asm/"
On 2016/01/06 19:09:57, adamk wrote: > On 2016/01/06 19:04:15, adamk wrote: > > On 2016/01/06 18:44:38, caitp wrote: > > > On 2016/01/06 18:15:49, caitp wrote: > > > > On 2016/01/06 17:47:50, Dan Ehrenberg wrote: > > > > > On 2016/01/06 at 17:46:50, Dan Ehrenberg wrote: > > > > > > On 2016/01/06 at 17:44:19, caitpotter88 wrote: > > > > > > > On 2016/01/06 17:39:10, commit-bot: I haz the power wrote: > > > > > > > > Try jobs failed on following builders: > > > > > > > > v8_linux64_asan_rel on tryserver.v8 (JOB_FAILED, > > > > > > > > > > > > > > > > > > > > > > > http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel/builds/...) > > > > > > > > > > > > > > hmmm --- --omit-quit seems to break $DONE in the test262 harness. > I'm > > > not > > > > > sure how that wouldn't have broken things before though > > > > > > > > > > > > What I would do is mark the right tests as PASS so the precq gets > > further, > > > > all > > > > > the way to ignition, and then see what happens in the trybot. > > > > > > > > > > Oh sorry wrong thread! > > > > > > > > The comment in d8.cc looks like --omit-quit is mainly for emscripten's > > > benefit. > > > > Maybe test262 runs on the asan bot don't need this flag. > > > > > > > > Maybe you are able to look at the v8_linux64_asan_rel recipe and update > the > > > > test262 flags? > > > > > > +svennepanne --- 9bb8b58549d543bacc230b4094ff5c196fa61ad3 adds --omit-quit, > > but > > > the test262 harness makes use of this function, especially to terminate > > > asynchronous tests. What other options do we have to actually run the > affected > > > tests? > > > > Sven no longer works on V8. It seems like we should be able to narrow the > > circumstances under which this flag is needed. > > E.g., you could add code to test/mjsunit/testcfg.py to add --omit-quit if the > path contains "/asm/" Hmm, but maybe you don't know you're running under ASAN in testcfg.py?
On 2016/01/06 19:10:26, adamk wrote: > On 2016/01/06 19:09:57, adamk wrote: > > On 2016/01/06 19:04:15, adamk wrote: > > > On 2016/01/06 18:44:38, caitp wrote: > > > > On 2016/01/06 18:15:49, caitp wrote: > > > > > On 2016/01/06 17:47:50, Dan Ehrenberg wrote: > > > > > > On 2016/01/06 at 17:46:50, Dan Ehrenberg wrote: > > > > > > > On 2016/01/06 at 17:44:19, caitpotter88 wrote: > > > > > > > > On 2016/01/06 17:39:10, commit-bot: I haz the power wrote: > > > > > > > > > Try jobs failed on following builders: > > > > > > > > > v8_linux64_asan_rel on tryserver.v8 (JOB_FAILED, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel/builds/...) > > > > > > > > > > > > > > > > hmmm --- --omit-quit seems to break $DONE in the test262 harness. > > I'm > > > > not > > > > > > sure how that wouldn't have broken things before though > > > > > > > > > > > > > > What I would do is mark the right tests as PASS so the precq gets > > > further, > > > > > all > > > > > > the way to ignition, and then see what happens in the trybot. > > > > > > > > > > > > Oh sorry wrong thread! > > > > > > > > > > The comment in d8.cc looks like --omit-quit is mainly for emscripten's > > > > benefit. > > > > > Maybe test262 runs on the asan bot don't need this flag. > > > > > > > > > > Maybe you are able to look at the v8_linux64_asan_rel recipe and update > > the > > > > > test262 flags? > > > > > > > > +svennepanne --- 9bb8b58549d543bacc230b4094ff5c196fa61ad3 adds > --omit-quit, > > > but > > > > the test262 harness makes use of this function, especially to terminate > > > > asynchronous tests. What other options do we have to actually run the > > affected > > > > tests? > > > > > > Sven no longer works on V8. It seems like we should be able to narrow the > > > circumstances under which this flag is needed. > > > > E.g., you could add code to test/mjsunit/testcfg.py to add --omit-quit if the > > path contains "/asm/" > > Hmm, but maybe you don't know you're running under ASAN in testcfg.py? run-tests.py adds --omit-quit if `options.asan` is set. If it can know the current suite is test262, it might be able to not add the flag (but then we miss leak tests for test262)
On 2016/01/06 19:13:28, caitp wrote: > On 2016/01/06 19:10:26, adamk wrote: > > On 2016/01/06 19:09:57, adamk wrote: > > > On 2016/01/06 19:04:15, adamk wrote: > > > > On 2016/01/06 18:44:38, caitp wrote: > > > > > On 2016/01/06 18:15:49, caitp wrote: > > > > > > On 2016/01/06 17:47:50, Dan Ehrenberg wrote: > > > > > > > On 2016/01/06 at 17:46:50, Dan Ehrenberg wrote: > > > > > > > > On 2016/01/06 at 17:44:19, caitpotter88 wrote: > > > > > > > > > On 2016/01/06 17:39:10, commit-bot: I haz the power wrote: > > > > > > > > > > Try jobs failed on following builders: > > > > > > > > > > v8_linux64_asan_rel on tryserver.v8 (JOB_FAILED, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel/builds/...) > > > > > > > > > > > > > > > > > > hmmm --- --omit-quit seems to break $DONE in the test262 > harness. > > > I'm > > > > > not > > > > > > > sure how that wouldn't have broken things before though > > > > > > > > > > > > > > > > What I would do is mark the right tests as PASS so the precq gets > > > > further, > > > > > > all > > > > > > > the way to ignition, and then see what happens in the trybot. > > > > > > > > > > > > > > Oh sorry wrong thread! > > > > > > > > > > > > The comment in d8.cc looks like --omit-quit is mainly for emscripten's > > > > > benefit. > > > > > > Maybe test262 runs on the asan bot don't need this flag. > > > > > > > > > > > > Maybe you are able to look at the v8_linux64_asan_rel recipe and > update > > > the > > > > > > test262 flags? > > > > > > > > > > +svennepanne --- 9bb8b58549d543bacc230b4094ff5c196fa61ad3 adds > > --omit-quit, > > > > but > > > > > the test262 harness makes use of this function, especially to terminate > > > > > asynchronous tests. What other options do we have to actually run the > > > affected > > > > > tests? > > > > > > > > Sven no longer works on V8. It seems like we should be able to narrow the > > > > circumstances under which this flag is needed. > > > > > > E.g., you could add code to test/mjsunit/testcfg.py to add --omit-quit if > the > > > path contains "/asm/" > > > > Hmm, but maybe you don't know you're running under ASAN in testcfg.py? > > run-tests.py adds --omit-quit if `options.asan` is set. If it can know the > current suite is test262, it might be able to not add the flag (but then we miss > leak tests for test262) added a fix for these, sort of
https://codereview.chromium.org/1568433004/diff/40001/test/test262/test262.st... File test/test262/test262.status (right): https://codereview.chromium.org/1568433004/diff/40001/test/test262/test262.st... test/test262/test262.status:596: # --omit-quit Could you file a bug and reference the bug from here?
The CQ bit was checked by caitpotter88@gmail.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/1568433004/#ps80001 (title: "Add a bug reference")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1568433004/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1568433004/80001
Message was sent while issue was closed.
Description was changed from ========== [promise] unskip more passing Test262 tests BUG=v8:4633 LOG=N R=adamk@chromium.org, littledan@chromium.org, cbruni@chromium.org ========== to ========== [promise] unskip more passing Test262 tests BUG=v8:4633 LOG=N R=adamk@chromium.org, littledan@chromium.org, cbruni@chromium.org ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001)
Message was sent while issue was closed.
Description was changed from ========== [promise] unskip more passing Test262 tests BUG=v8:4633 LOG=N R=adamk@chromium.org, littledan@chromium.org, cbruni@chromium.org ========== to ========== [promise] unskip more passing Test262 tests BUG=v8:4633 LOG=N R=adamk@chromium.org, littledan@chromium.org, cbruni@chromium.org Committed: https://crrev.com/c12a47a52059dc9464b14d2276f176491eebd9cf Cr-Commit-Position: refs/heads/master@{#33160} ==========
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/c12a47a52059dc9464b14d2276f176491eebd9cf Cr-Commit-Position: refs/heads/master@{#33160} |
