Closed johnjbarton closed 9 years ago
Did you mean runInContext
, runInThisContext
or runInNewContext
?
Whichever one you are using it looks like you are reusing a context since that is the only way that typeof a
can be number
on line 5.
I'm not surprised that we somehow polluted the global space with values a, b,c, but I am surprised that combination of "use strict" and var a; did not save us.
'use strict'
only reports error if you try to read or write to a non declared binding and there is no property on the global object.
'use strict';
global.x = 1;
x = 2; // OK
y = 3; // ReferenceError
Sorry, runInThisContext
. For module loading I think that is correct. It's actually the current behavior that confuses me. If some other test says var a = 1;
(and that seems likely and what I observe now), then how can a
be undefined in Simplify.js
?
Shouldn't we be running these tests in IIFE or loading them as modules anyway?
It's possible vm and/or V8 has bugs here :-/. Minimal repros appreciated. These seem potentially related. https://github.com/nodejs/io.js/issues/769 https://github.com/joyent/node/issues/9084 https://github.com/nodejs/io.js/issues/548 Especially https://github.com/joyent/node/issues/9084#issuecomment-75099054
Shouldn't we be running these tests in IIFE or loading them as modules anyway?
Definitely not. Script code has different semantics than Module and FunctionBody.
Simplify.js: Global vars creates properties on the global (in Script).
What we probably should do is to create a new context for every test but I am concerned about the performance of doing that.
Context creation is much faster in io.js 2.0.2 onward now that snapshots have been re-enabled.
Sure. But nothing is going to beat global eval. Still, if the performance regression of the tests is acceptable it would be a lot better to use new contexts to remove interdependencies.
I'm still unclear. Consider this Script:
"use strict"
var b = 'b';
Followed by this one:
"use strict"
var a, b;
a = 5;
assert.isUndefined(b);
It's my understanding that after the second script completes we should assert. If we execute these in the other order, we should not assert. Thus I believe the test is incorrect: we can't assert b is undefined because we don't know the status of the global b
.
I agree with you. The tests sometimes break due to changes in other files. When that happens a more unique name is usually used.
In the case of the Simplify test, the assert.isUndefined(b);
is really trying to verify that we did not assign to b
in the test. So I think we can initialize the test variables to a unique value and check that instead.
I've reworked the feature test harness into es6 but one of the tests won't pass. It transcodes to:
Here is the output
The new version uses V8
runInTheContext
which purports to resemble(0,eval)('code')
, essentially the call used in the older version. It seems this isn't quite true.I'm not surprised that we somehow polluted the global space with values
a, b,c
, but I am surprised that combination of"use strict"
andvar a;
did not save us.Anyway any suggestions? Of course I could force the tests to run again with
eval()
but I was hoping to use the same code for all the node evaluations.