Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Supporting asynchronous tests #24

Closed
juandopazo opened this issue Jan 28, 2014 · 12 comments
Closed

Supporting asynchronous tests #24

juandopazo opened this issue Jan 28, 2014 · 12 comments
Labels

Comments

@juandopazo
Copy link
Contributor

In order to test module loaders and promises, test262 needs to support asynchronous tests.

The most straightforward way would be to extend the signature of runTestCase(fn) The test case function could get a done function as a parameter, similar to what Mocha does.

I think this would introduce the least increase to the API surface.

An open issue is what to do with tests that time out, because test262 should not know about setTimeout.

@domenic @kriskowal would you mind commenting on this?

@kriskowal
Copy link
Member

It is certainly the norm for test harnesses to pass a done(?error) method and require it to be called by the test only if testFn.length >= 1. The pattern composes well and addresses the needs of testing promises themselves. For higher layers, it is useful if the scaffold will check whether testFn returns an object, and if so, treat it as a promise for the completion of the test.

@bterlson
Copy link
Member

ping @anba, who may have an opinion on augmenting runTestCase further...

@anba
Copy link
Contributor

anba commented Jan 29, 2014

Mocha uses a setTimeout based solution for its asynchronous tests, so, as already mentioned in #comment 0, this approach will not work for test262. It's kind of difficult to properly test Promise support in ECMAScript implementations, because task queues are a completely new concept in ECMAScript and there are no existing tools to track eventual computations. For example the JavaScriptCore Promise tests rely on comparing console output. And the V8 tests use Object.observe() and some internal functions.

My first idea was to use the @negative attribute and hope implementations use the same uncaught exception handling in NextTask as for other uncaught exceptions, that means to stop script execution. Unfortunately the V8 implementation silently ignores exceptions in NextTask 😞.

/**
 * @negative TaskExecuted
 */

var TaskExecuted = new Error("TaskExecuted");

// Function to throw an exception from within "8.4.2 NextTask"
function stopExecution() {
  let p = Promise.reject();
  p.constructor = function(r) {
    r(() => {}, () => { throw TaskExecuted });
  };
  p.then();
}

try {
  Promise.resolve().then(stopExecution);
} catch (e) {
  // catch any error here to ensure `throw` from stopExecution()
  // is executed after current turn
}

Related:

@juandopazo
Copy link
Contributor Author

I'l start sketching the promise tests as if runTestCase took a function argument with a done optional parameter. I'll probably have them done quite a bit after we figure out how to deal with tasks.

Clarifying:

runTestCase(function () {
 // looks at the return value
});

runTestCase(function (done) {
  // waits for `done` to be called
  // If `done` is called with a non-undefined first parameter
  // it is considered the same as calling $ERROR
});

@bterlson
Copy link
Member

I'm not sure I would prefer runTestCase over something like

/**
 * @async
 */

doSomethingAsync().then(function() {
    $DONE();
});

Where the harness will wait for $DONE to be called when the @async attribute is present, with some timeout value (implementing timeout without setTimeout is likely difficult but possible I believe).

I say this because I think the current guidance is to effectively deprecate runTestCase, so it would be a shame to resurrect it for async if this is true.

@juandopazo
Copy link
Contributor Author

I'd be ok with that too, though I find it a bit weird to be using promises to test promises. It seems like simplicity should win here. Also promises may catch unexpected errors.

@bterlson
Copy link
Member

The only harness requirement is $DONE function and support for @async attribute. In the example above, consider it a test for "promise invokes .then handler" or something... IOW I think all the promise machinery is under test. I actually think the two proposed implementations are mostly identical, the only real difference being whether $DONE/done are globally exposed or passed as a param to the runTestCase callback. @juandopazo, is that your understanding as well?

@juandopazo
Copy link
Contributor Author

Oh I got it. My thought was that having done as a parameter would communicate by itself that the test is async without needing the async attribute. But I'm ok either way.

@bterlson
Copy link
Member

I suppose the harness could search for $DONE in the test case somewhere to know that it's async... probably a decent idea, but I haven't thought too hard about it :)

@domenic
Copy link
Member

domenic commented Aug 13, 2014

This was fixed by #34

@smikes
Copy link
Contributor

smikes commented Oct 24, 2014

I believe this is fixed now, and there is now async test support in:

  • python console runner
  • node test262-harness console runner
  • browser runner

Comments, @bterlson @juandopazo

@bterlson
Copy link
Member

Agreed, closing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants