New issue
Advanced search Search tips
Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 17
Cc:
HW: ----
NextAction: ----
OS: ----
Priority: ----
Type: FeatureRequest

Blocking:
issue 896



Sign in to add a comment
link

Issue 8467: Improving V8’s coverage for early errors in Test262

Reported by mathias@chromium.org, Nov 15 Project Member

Issue description

Design doc: go/v8-test262-early

The upstream Test262 work for this has recently been completed. We can now move forward with updating the V8 test runner.

sergiyb@ agreed to work on a CL adding a new FAIL_PHASE_ONLY outcome to our test runner that essentially has the following effect. Instead of:

    d8 [options] test262/data/…/test-file.js

Tests with this special outcome would run:

    d8 [options] harness-adapt-donotevaluate.js test262/data/…/test-file.js

Where harness-adapt-donotevaluate.js contains nothing but:

    $DONOTEVALUATE = () => {};
 

Comment 1 by serg...@chromium.org, Nov 15

CL implementing FAIL_PHASE_ONLY modifier: https://crrev.com/c/1338347

Comment 2 by mathias@chromium.org, Nov 16

Cc: adamk@chromium.org gsat...@chromium.org

Comment 3 by mathias@chromium.org, Nov 16

Cc: machenb...@chromium.org

Comment 4 by bugdroid1@chromium.org, Nov 17

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/eb93a84632cdc2f96808d67dffcf7e05fad1aff8

commit eb93a84632cdc2f96808d67dffcf7e05fad1aff8
Author: Mathias Bynens <mathias@chromium.org>
Date: Sat Nov 17 03:04:01 2018

Roll Test262

This roll should cover the last batch of upstream $DONOTEVALUATE
updates.

TBR=gsathya@chromium.org

Bug: v8:7834,  v8:8467 
Change-Id: Ia1c6e8fa2fd7fd020c5499b3825a8c1e6c14db47
Reviewed-on: https://chromium-review.googlesource.com/c/1338348
Reviewed-by: Mathias Bynens <mathias@chromium.org>
Commit-Queue: Mathias Bynens <mathias@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57587}
[modify] https://crrev.com/eb93a84632cdc2f96808d67dffcf7e05fad1aff8/DEPS

Comment 6 by mathias@chromium.org, Nov 27

The remaining work is to make each FAIL_PHASE_ONLY test run twice:

- run it once with the original $DONOTEVALUATE, and treat the outcome as FAIL
- run it once with $DONOTEVALUATE patched out, and treat the outcome as PASS

That way, if a test is marked FAIL_PHASE_ONLY and we fix the behavior to now throw during parsing, it would fail our Test262 run (until we update FAIL_PHASE_ONLY to PASS).

Comment 7 by serg...@chromium.org, Nov 28

I believe the other remaining work item is to actual make use of this flag and mark all failing tests as such in the statusfile.

Comment 8 by mathias@chromium.org, Nov 28

Indeed. I’ll happily take care of that once the work in comment #6 is completed.

Comment 9 by machenb...@chromium.org, Nov 29

The stuff in 6 I assume can be solved locally in test262's testcfg.py? We can create different runs similar to how we do it with strict/sloppy.

Comment 10 by serg...@chromium.org, Dec 4

I assume you suggest using test262's VariantGenenator for this: https://crrev.com/c/1361164.

Comment 11 by bugdroid1@chromium.org, Dec 13

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/996cf21954f8692665c016749aace80b2f3665d9

commit 996cf21954f8692665c016749aace80b2f3665d9
Author: Sergiy Belozorov <sergiyb@chromium.org>
Date: Thu Dec 13 11:28:57 2018

[tools] Generate additional variant for FAIL_PHASE_ONLY tests

The additional variant does not use wrapper disabling phase tests and negated
outcome processor. This allows to ensure that tests marked FAIL_PHASE_ONLY, do
actually fail without it.

R=machenbach@chromium.org

Bug:  v8:8467 
Change-Id: I66e07bd7107520872cc013bf0f33fdc6664baf56
Reviewed-on: https://chromium-review.googlesource.com/c/1361164
Commit-Queue: Sergiy Belozorov <sergiyb@chromium.org>
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58211}
[modify] https://crrev.com/996cf21954f8692665c016749aace80b2f3665d9/test/test262/testcfg.py
[modify] https://crrev.com/996cf21954f8692665c016749aace80b2f3665d9/tools/testrunner/objects/testcase.py
[modify] https://crrev.com/996cf21954f8692665c016749aace80b2f3665d9/tools/testrunner/outproc/test262.py

Comment 12 by mathias@chromium.org, Dec 13

Blockedon: 7828

Comment 13 by mathias@chromium.org, Dec 13

Blockedon: -7828

Comment 14 by bugdroid1@chromium.org, Dec 13

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/f605302e3322ca4d1a533a00d34783d3aae9b906

commit f605302e3322ca4d1a533a00d34783d3aae9b906
Author: Mathias Bynens <mathias@qiwi.be>
Date: Thu Dec 13 15:52:28 2018

[test] Mark early RegExp errors as FAIL_PHASE_ONLY

This ensures V8 at least throws the correct exception for these
tests, even if it happens at the wrong phase (i.e. at runtime
instead of at parse time).

Bug:  v8:8467 
Change-Id: I101aa2c7e073a56163c29e96e6c47f6ff0a01e53
Reviewed-on: https://chromium-review.googlesource.com/c/1375909
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Commit-Queue: Mathias Bynens <mathias@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58226}
[modify] https://crrev.com/f605302e3322ca4d1a533a00d34783d3aae9b906/test/test262/test262.status

Comment 15 by mathias@chromium.org, Dec 17

Status: Fixed (was: Assigned)
Special handling for the FAIL_PHASE_ONLY outcome has been implemented on the V8 side, and we’re now using it to improve test coverage for early regular expression errors. \o/

Closing this issue.

Comment 16 by mathias@chromium.org, Feb 6

Blocking: 896

Sign in to add a comment