Refactor directive prologue tests for parsers #1592

jugglinmike · 2018-06-11T00:12:39Z

@leobalter @rwaldron This is in service of gh-1356. The tests modified in this patch have a trait that differentiates them from the tests we've previously reformatted for the benefit of parsers. Because they concern the observance of strict mode, there are parsing and runtime behaviors that we need to verify. So while in those previous patches, we could just replace the calls to eval with direct program code, in this case, I've taken care to preserve assertions for runtime semantics. That said, I've still modified those tests in order to make this consideration more clear (they now test for strict mode through assignment instead of eval).

To facilitate review (and help verify that I didn't invalidate any of the old tests), I've split this work into a number of distinct commits. Hope that helps!

Promote consistency in coverage by adding new tests that correspond to those that were authored previously.

Verify runtime semantics through assignment to an unresolvable reference, reducing the complexity of tests that previously relied on the semantics of the `eval` function.

Test262 already includes tests to ensure the correct runtime semantics for these forms. Add equivalent tests designed to verify that the equivalent parsing behavior is also observed.

rwaldron · 2018-06-13T15:36:44Z

test/language/directive-prologue/func-decl-no-semi-parse.js

+// This code is governed by the BSD license found in the LICENSE file.
+
+/*---
+es5id: 10.1.1-2-s


Add or replace es5id with:

esid: use-strict-directive

rwaldron · 2018-06-13T15:37:40Z

test/language/directive-prologue/func-decl-inside-func-decl-parse.js

+// This code is governed by the BSD license found in the LICENSE file.
+
+/*---
+es5id: 10.1.1-15-s


Add or replace es5id with:

esid: sec-strict-mode-code

rwaldron · 2018-06-13T15:38:08Z

test/language/directive-prologue/func-decl-no-semi-runtime.js

+// This code is governed by the BSD license found in the LICENSE file.
+
+/*---
+es5id: 10.1.1-2-s


Add or replace es5id with:

esid: use-strict-directive

rwaldron · 2018-06-13T15:38:51Z

test/language/directive-prologue/func-decl-no-semi-runtime.js

+// This code is governed by the BSD license found in the LICENSE file.
+
+/*---
+es5id: 10.1.1-2-s


Add or replace es5id with:

esid: use-strict-directive

rwaldron · 2018-06-13T15:39:06Z

test/language/directive-prologue/func-decl-parse.js

+// This code is governed by the BSD license found in the LICENSE file.
+
+/*---
+es5id: 10.1.1-19-s


Add or replace es5id with:

esid: sec-strict-mode-code

rwaldron · 2018-06-13T15:39:16Z

test/language/directive-prologue/func-expr-inside-func-decl-parse.js

+// This code is governed by the BSD license found in the LICENSE file.
+
+/*---
+es5id: 10.1.1-16-s


Add or replace es5id with:

esid: sec-strict-mode-code

rwaldron · 2018-06-13T15:39:21Z

test/language/directive-prologue/func-expr-no-semi-parse.js

+// This code is governed by the BSD license found in the LICENSE file.
+
+/*---
+es5id: 10.1.1-2-s


Add or replace es5id with:

esid: sec-strict-mode-code

rwaldron · 2018-06-13T15:39:26Z

test/language/directive-prologue/func-expr-parse.js

+// This code is governed by the BSD license found in the LICENSE file.
+
+/*---
+es5id: 10.1.1-22-s


Add or replace es5id with:

esid: sec-strict-mode-code

rwaldron · 2018-06-13T15:39:34Z

test/language/directive-prologue/set-accsr-runtime.js

+// This code is governed by the BSD license found in the LICENSE file.
+
+/*---
+es5id: 10.1.1-25-s


Add or replace es5id with:

esid: sec-strict-mode-code

jugglinmike · 2018-06-18T00:38:24Z

@rwaldron Metadata added!

jugglinmike · 2018-06-18T00:57:54Z

It looks like the linter has a faulty rule. There are a number of ways we could address this:

add exceptions for the new files to the linter's white list
extend the regular expression with a special case for this particular esid value
remove the linting rule
relax the linting rule to simply assert the value of esid satisfies the general requirements for HTML document fragment identifiers
extend the linter to fetch the spec and scrape out the value identifiers

I'm happy to help out with any of these, though my preference would be the fourth. @rwaldron what do you think?

rwaldron · 2018-06-20T14:38:02Z

jugglinmike#2

leobalter · 2018-06-20T15:13:37Z

test/language/directive-prologue/func-decl-final-runtime.js

-            eval("var public = 1;");
-            assert.sameValue(public, 1);
+            test262unresolvable = null;
+            assert.sameValue(test262unresolvable, null);
            "use strict";


I wonder if we have some coverage for the pragma only preceded by an empty statement. I did not find anything from a quick grep.

function fun() { ;'use strict'; unr = null; assert.sameValue(unr, null); } fun();

leobalter · 2018-06-20T15:21:28Z

relax the linting rule to simply assert the value of esid satisfies the general requirements for HTML document fragment identifiers

I'm positive with this solution.

jugglinmike · 2018-07-08T19:33:59Z

To unblock this, I've submitted #1632 (as an alternative to jugglinmike#2).

rwaldron · 2018-07-13T18:13:16Z

I've rebased this locally and confirmed that linting passes:

rwaldron in ~/clonez/test262 on refactor-for-parsers-directive
$ ./tools/lint/lint.py --whitelist lint.whitelist test/language/directive-prologue/
Linting 62 files
Linting complete. 0 errors found.

rwaldron · 2018-07-13T18:14:08Z

Merged locally, pushed to master. Thanks!

jugglinmike added 4 commits June 10, 2018 18:58

Rename tests for clarity

86a6c08

Normalize coverage

172c494

Promote consistency in coverage by adding new tests that correspond to those that were authored previously.

Simplify tests for runtime semantics

cd2ffd2

Verify runtime semantics through assignment to an unresolvable reference, reducing the complexity of tests that previously relied on the semantics of the `eval` function.

Add tests for parsing behavior

e95c75b

Test262 already includes tests to ensure the correct runtime semantics for these forms. Add equivalent tests designed to verify that the equivalent parsing behavior is also observed.

jugglinmike requested review from rwaldron and leobalter June 11, 2018 00:12

rwaldron requested changes Jun 13, 2018

View reviewed changes

jugglinmike added 3 commits June 17, 2018 20:34

Extend metadata with esid

d8c5d23

fixup! Normalize coverage

c90f799

fixup! Add tests for parsing behavior

90c667a

leobalter reviewed Jun 20, 2018

View reviewed changes

jugglinmike mentioned this pull request Jul 8, 2018

Relax linting rules for esid metadata #1632

Merged

rwaldron closed this Jul 13, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor directive prologue tests for parsers #1592

Refactor directive prologue tests for parsers #1592

jugglinmike commented Jun 11, 2018

rwaldron Jun 13, 2018

rwaldron Jun 13, 2018

rwaldron Jun 13, 2018

rwaldron Jun 13, 2018

rwaldron Jun 13, 2018

rwaldron Jun 13, 2018

rwaldron Jun 13, 2018

rwaldron Jun 13, 2018

rwaldron Jun 13, 2018

jugglinmike commented Jun 18, 2018

jugglinmike commented Jun 18, 2018

rwaldron commented Jun 20, 2018

leobalter Jun 20, 2018

leobalter commented Jun 20, 2018

jugglinmike commented Jul 8, 2018

rwaldron commented Jul 13, 2018

rwaldron commented Jul 13, 2018 •

edited

Loading

Refactor directive prologue tests for parsers #1592

Refactor directive prologue tests for parsers #1592

Conversation

jugglinmike commented Jun 11, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jugglinmike commented Jun 18, 2018

jugglinmike commented Jun 18, 2018

rwaldron commented Jun 20, 2018

Choose a reason for hiding this comment

leobalter commented Jun 20, 2018

jugglinmike commented Jul 8, 2018

rwaldron commented Jul 13, 2018

rwaldron commented Jul 13, 2018 • edited Loading

rwaldron commented Jul 13, 2018 •

edited

Loading