Skip to content

Add non language tests#10

Merged
nicolo-ribaudo merged 1 commit intobabel:masterfrom
jbhoosreddy:master
Oct 14, 2019
Merged

Add non language tests#10
nicolo-ribaudo merged 1 commit intobabel:masterfrom
jbhoosreddy:master

Conversation

@jbhoosreddy
Copy link
Copy Markdown
Collaborator

@jbhoosreddy jbhoosreddy commented Oct 6, 2019

Summary of changes

  1. Remove non-language tests
  2. Removes chalk as a dependency.

@jbhoosreddy

This comment has been minimized.

@nicolo-ribaudo
Copy link
Copy Markdown
Member

If you apply this patch, it should work:

diff --git a/lib/run-tests/babel-agent.js b/lib/run-tests/babel-agent.js
index 7a95cb0..b7304f3 100644
--- a/lib/run-tests/babel-agent.js
+++ b/lib/run-tests/babel-agent.js
@@ -8,11 +8,11 @@ const transpile = require("./transpile");
 
 const CORE_JS = require.resolve("core-js-bundle");
 const REGENERATOR = require.resolve("regenerator-runtime/runtime");
-const INTL = require.resolve('./intl-polyfill');
+const INTL = require.resolve("intl/dist/Intl");
 const POLYFILLS = `[
   fs.readFileSync(${JSON.stringify(CORE_JS)}, "utf8"),
   fs.readFileSync(${JSON.stringify(REGENERATOR)}, "utf8"),
-  fs.readFileSync(${JSON.stringify(INTL)}, "utf8")
+  "self = window = this;" + fs.readFileSync(${JSON.stringify(INTL)}, "utf8") + "; delete self; delete window;"
 ]`;

The problem is that require doesn't work in ESHost contexts, because it's node-specific.

Note that the intl polyfill is far from being spec compliant:

# Run 594 out of 69345 tests.
# Passed 114 out of 594 tests.

I would prefer to simply skip those tests.

Also, as I explained on Slack, ignoredFeatures doesn't do what you think in @babel/parser: it doesn't mean "don't run these tests", but it means "don't warn if these features don't have a corresponding parser plugin".

@jbhoosreddy
Copy link
Copy Markdown
Collaborator Author

while I have a PR open, I wanted to ask some help from @rwaldron regarding failures like:

test/built-ins/Array/proto-from-ctor-realm.js strict mode (expected success, got runtime error)
Error: test/built-ins/Array/proto-from-ctor-realm.js strict mode (expected success, got runtime error)
    at ReferenceError: $262 is not defined
    at /home/circleci/babel/babel-test262-runner/lib/run-tests/index.js:59:11
    at undefined:163:13
    at Function.vm.runInESHostContext (/tmp/axDDGOhmTHAhIzS8Ca1F/f-1570330901281-7718-1d9kul3.t6u5.js:17:13)
    at Object.<anonymous> (/tmp/axDDGOhmTHAhIzS8Ca1F/f-1570330901281-7718-1d9kul3.t6u5.js:28:4)
    at Module._compile (module.js:456:26)
    at Object.Module._extensions..js (module.js:474:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Function.Module.runMain (module.js:497:10)
    at startup (node.js:119:16)
    at node.js:945:3

in https://app.circleci.com/jobs/github/babel/babel/11859/tests

Here's how Test262Stream is currently initialized: https://github.com/babel/babel-test262-runner/blob/master/lib/run-tests/index.js#L23

cc. @leobalter, @jugglinmike or someone who might know.

@rwaldron
Copy link
Copy Markdown

When I use a custom preprocessor with test262-harness, I don't encounter this issue. The problem that I'm seeing here is that you're trying to use the NodeAgent directly, but not passing the options it needs to correctly transform the "runtime" code. Until now, the only consumers that were using eshost for Test262 test running and Test262 host environment creation were eshost-cli and test262-harness, and both of them explicitly include shortName: '$262' when they instantiate eshost agents via the createAgent factory. I'm only noticing now that it's not documented. In lib/run-tests/agent-pool.js, change:

const agent = new BabelAgent({ hostPath });

To:

const agent = new BabelAgent({ hostPath, shortName: '$262' });

... That will eliminate the error you reported above. In the meantime, I'm going to work on making that the default identifier used in all built-in runtimes.


I've noticed that this approach will transpile the harness code as well, which is probably not a problem in practice, but I wouldn't consider the results of this runner to be valid Test262 conformance tests, because that's a violation of the versioned host requirements defined in INTERPRETING.md.

@nicolo-ribaudo
Copy link
Copy Markdown
Member

I've noticed that this approach will transpile the harness code as well

Isn't that expected? "Evaluate this code using Babel" means "Transpile this code using Babel, and then evaluate it using the underlying engine". I don't see why the harness code should be evaluated differently.

@jbhoosreddy jbhoosreddy force-pushed the master branch 3 times, most recently from 4c2e00d to 7d85817 Compare October 14, 2019 22:00
@jbhoosreddy jbhoosreddy changed the title Add ignored features & attempt polyfillying Intl Add non language tests Oct 14, 2019
module.exports = function shouldIgnore(test) {
if (test.file.startsWith("test/intl402")) return true;
if (test.attrs.features && test.attrs.features.includes("Proxy")) return true;
if (!test.file.startsWith("test/language")) return true;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hopefully this will make the test suite a lot faster.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know right! I was thinking the same thing!

@nicolo-ribaudo nicolo-ribaudo merged commit 1c6072f into babel:master Oct 14, 2019
@rwaldron
Copy link
Copy Markdown

Isn't that expected?

I wouldn't expect that at all. I would expect Babel to transpile only the test source itself. Other host runtimes don't modify the harness code, so why should Babel?

@nicolo-ribaudo
Copy link
Copy Markdown
Member

nicolo-ribaudo commented Oct 15, 2019

They handle harness code exactly like they handle tests code: they execute it.
The fact that, when using Babel, "executing code" produces an intermediate representation in JavaScript rather than immediately generating bytecode is an implementation detail.

@jugglinmike
Copy link
Copy Markdown

The interpreting guidelines allow only very specific modifications to any code (harness code or test code). Babel's transformations aren't endorsed. I'm guessing that modifying the guidelines to allow Babel's usage would be kind of tricky. Given that Babel ultimately relies on a traditional runtime to actually evaluate code, it might not be possible to really call Babel "conforming" in the strict official TC39 sense. That distinction isn't necessary for Babel and its users to benefit from Test262, so it might be best to let it go until there's some dispute about correctness.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants