type coercion harness utilities + features flags + linting #1221
Conversation
…"features: ..." tags
… deps that require a feature flag
| let ieval = eval; | ||
|
|
||
| assert.throws(err, function() { Function(wrappedCode); }, `Function: ${code}`); | ||
| assert.throws(err, function() { Function(wrappedCode); }, 'Function: ' + code); |
There was a problem hiding this comment.
Sorry @leobalter ;)
| @@ -20,5 +20,5 @@ function compareArray(a, b) { | |||
|
|
|||
| assert.compareArray = function(actual, expected, message) { | |||
| assert(compareArray(actual, expected), | |||
| `Expected [${actual.join(", ")}] and [${expected.join(", ")}] to have the same contents. ${message}`); | |||
| 'Expected [' + actual.join(', ') + '] and [' + expected.join(', ') + '] to have the same contents. ' + message); | |||
There was a problem hiding this comment.
Sorry again @leobalter ;)
| } | ||
| } | ||
|
|
||
| if (Object.prototype.hasOwnProperty.call(desc, 'configurable')) { | ||
| if (desc.configurable !== originalDesc.configurable || | ||
| desc.configurable !== isConfigurable(obj, name)) { | ||
| failures.push(`descriptor should ${desc.configurable ? '' : 'not '}be configurable`); | ||
| failures.push('descriptor should ' + (desc.configurable ? '' : 'not ') + 'be configurable'); | ||
| } |
There was a problem hiding this comment.
Ok, @leobalter I'm not really sorry ;)
…st is invalid anyway!
rwaldron
force-pushed
the
thejoshwolfe-type-coercion
branch
from
b22899a to
e294b80
Compare
| @@ -51,6 +51,7 @@ regexp-unicode-property-escapes | |||
|
|
|||
| # Shared Memory and atomics | |||
| # https://github.com/tc39/ecmascript_sharedmem | |||
| Atomics | |||
There was a problem hiding this comment.
Does it really make sense to have feature flags for parts of proposals? Atomics is part of the SharedArrayBuffer proposal. Do we expect implementations to do part of a language feature and separate it out this way?
There was a problem hiding this comment.
I'm +1 to use a single tag for each proposal, some exceptions can be considered, but in this case I believe it's ok to use the SharedArrayBuffer tag.
Wrt the exceptions, I guess this could possibly apply to private class fields, but we should postpone this discussion to a later time.
There was a problem hiding this comment.
I added Atomics just to be thorough, since it's a new global. Is the consensus here that I should remove?
There was a problem hiding this comment.
I'm in favor of using a single name for features from the SharedArrayBuffer proposal. Even though, I believe we can change it as a follow up improvement.
If this is good to land, I don't want to block it, we can have a separate discussion to find an agreement for this specific part later.
|
Is there a reason we're putting the |
|
@thejoshwolfe The harness files aren't test files, so they won't have the same frontmatter in general. Harness files are included in other files, so different logic is needed here. It might not be the prettiest, but it seems like the current system is a working solution. |
Dan's response is 100%, but I'll add some explanation re: the "frontmatter" that appears in the harness files: it's meaningless. I recently added that information just to keep track of what was in each file, I could've used jsdoc or similar—it wouldn't have made a difference. |
|
@thejoshwolfe that's an interesting point, and Dan is correct. The harness files are conceptually replaceable, they only provide an API that is expected by the tests and those can run with anything similar. There are even examples of expected replacements, like |
This builds on #1200 as described in #1215 (comment)
It's likely better to review this one commit at a time: