Skip to content

Commit

Permalink
Tests: Enable test/main/promise.js in mozjs CI job
Browse files Browse the repository at this point in the history
It just had one inline call to setTimeout in one of the test cases,
which is easily changed to use our local `defer` function instead.
After that, it passed.

While at it, simplify and synchronise the two mock-promise functions.
Not yet worth abstracting I think, but thinking about it.

Ref #1511.
  • Loading branch information
Krinkle committed Jun 7, 2021
1 parent 0c2bb7f commit bea9feb
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 21 deletions.
17 changes: 17 additions & 0 deletions .github/workflows/CI.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,23 @@ jobs:
- name: Tests
run: ${{ matrix.script }}

# To reproduce SpiderMonkey builds locally:
#
# 1. Run the build the same way you normally do.
# ```
# nobody@nodejs-isolated:/qunit$ npm run build
# ```
# 2. Start a temporary Docker container using the official Ubuntu image from DockerHub,
# and mount the current working directory in it:
# ```
# you@host:/qunit$ MNT=$(basename "$PWD"); docker run --rm --interactive --tty --mount type=bind,source="$PWD",target="/$MNT",readonly --entrypoint /bin/sh ubuntu:focal -c "cd /$MNT;bash"
# ```
# 3. Run the following from the Docker container's bash prompt:
# ```
# root@ubuntu-tmp/qunit$ apt-get update && DEBIAN_FRONTEND=noninteractive apt-get install -y libmozjs-68-dev
# root@ubuntu-tmp/qunit$ js68 test/mozjs.js
# ```
#
sm-test:
name: SpiderMonkey
runs-on: ubuntu-20.04
Expand Down
19 changes: 6 additions & 13 deletions test/main/assert.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,11 @@
function buildMockPromise( settledValue, shouldFulfill ) {

// Support IE 9: Promise not supported, test should not load polyfil globally to ensure
// QUnit works without it, so we create our own thenable with setTimeout.
// Support SpiderMonkey: setTimeout is not supported, re-use the IE 9 mock, but
// using SM's native Promise support for the deferred callback handling.
var defer = typeof setTimeout !== "undefined" ?
setTimeout :
function( fn ) {
Promise.resolve().then( fn );
};
// Support IE 9: Promise not supported, test MUST NOT load polyfil globally.
// Support SpiderMonkey: setTimeout is not supported, but native Promise is.
var defer = typeof setTimeout !== "undefined" ? setTimeout : function( fn ) {
Promise.resolve().then( fn );
};

// Return a mock self-fulfilling Promise ("thenable")
var thenable = {
then: function( fulfilledCallback, rejectedCallback ) {
defer( function() {
Expand All @@ -19,12 +14,10 @@ function buildMockPromise( settledValue, shouldFulfill ) {
rejectedCallback.call( thenable, settledValue );
} );

// returning another thennable for easy confirmation
// of return value
// returning another thenable for easy confirmation of return value
return buildMockPromise( "final promise", true );
}
};

return thenable;
}

Expand Down
17 changes: 10 additions & 7 deletions test/main/promise.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
// Support IE 9: Promise not supported, test MUST NOT load polyfil globally.
// Support SpiderMonkey: setTimeout is not supported, but native Promise is.
var defer = typeof setTimeout !== "undefined" ? setTimeout : function( fn ) {
Promise.resolve().then( fn );
};

// NOTE: Adds 1 assertion
function createMockPromise( assert, reject, value ) {
if ( arguments.length < 3 ) {
value = {};
}

// Return a mock self-fulfilling Promise ("thenable")
var thenable = {
then: function( fulfilledCallback, rejectedCallback ) {
assert.strictEqual( this, thenable, "`then` was invoked with the Promise as the " +
"context" );
setTimeout( function() {
assert.strictEqual( this, thenable, "`then` invoked with our Promise as thisValue" );
defer( function() {
return reject ?
rejectedCallback.call( thenable, value ) :
fulfilledCallback.call( thenable, value );
Expand Down Expand Up @@ -150,10 +153,10 @@ QUnit.module( "Support for Promise", function() {
assert.expect( 2 );

var done = assert.async();
setTimeout( function() {
defer( function() {
assert.true( true );
done();
}, 100 );
} );

// Adds 1 assertion
return createMockPromise( assert );
Expand Down
2 changes: 1 addition & 1 deletion test/mozjs.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ loadRelativeToScript( "../test/main/assert.js" );
loadRelativeToScript( "../test/main/assert/step.js" );
// Requires setTimeout, loadRelativeToScript( "../test/main/assert/timeout.js" );
// Requires setTimeout, loadRelativeToScript( "../test/main/async.js" );
// Requires setTimeout, loadRelativeToScript( "../test/main/promise.js" );
loadRelativeToScript( "../test/main/promise.js" );
loadRelativeToScript( "../test/main/dump.js" );
// Requires setTimeout, loadRelativeToScript( "../test/main/modules.js" );
loadRelativeToScript( "../test/main/deepEqual.js" );
Expand Down

0 comments on commit bea9feb

Please sign in to comment.