Skip to content

Commit

Permalink
asyncHelpers tests: Address code review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
cjtenny authored and ptomato committed Feb 21, 2023
1 parent e0d5b78 commit eb44f67
Show file tree
Hide file tree
Showing 18 changed files with 50 additions and 58 deletions.
2 changes: 1 addition & 1 deletion test/harness/assert-throws-same-realm.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
/*---
description: >
Functions that throw instances of the realm specified constructor function
satisfy the assertion, without cross realms collisions.
do not satisfy the assertion with cross realms collisions.
---*/

var intrinsicTypeError = TypeError;
Expand Down
4 changes: 3 additions & 1 deletion test/harness/asyncHelpers-asyncTest-func-throws-sync.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@ description: |
includes: [asyncHelpers.js]
---*/
var called = false;
var msg = "Should not be rethrown";
function $DONE(error) {
called = true;
assert(error instanceof Test262Error);
assert.sameValue(error.message, msg, "Should report correct error");
}
asyncTest(function () {
throw new Test262Error("Should not be rethrown");
throw new Test262Error(msg);
});
assert(called, "asyncTest called $DONE with a synchronously thrown error");
1 change: 1 addition & 0 deletions test/harness/asyncHelpers-asyncTest-return-not-thenable.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ includes: [asyncHelpers.js, compareArray.js]

const doneValues = [];
function $DONE(error) {
// Will be a TypeError from trying to invoke .then() on non-thenable
doneValues.push(error instanceof TypeError);
}
asyncTest(function () {
Expand Down
2 changes: 1 addition & 1 deletion test/harness/asyncHelpers-asyncTest-returns-undefined.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// This code is governed by the BSD license found in the LICENSE file.
/*---
description: |
The 'asyncTest' helper when called with async flag always returns to undefined.
The 'asyncTest' helper when called with async flag always returns undefined.
flags: [async]
includes: [asyncHelpers.js]
---*/
Expand Down
3 changes: 2 additions & 1 deletion test/harness/asyncHelpers-asyncTest-without-async-flag.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ includes: [asyncHelpers.js]
function makePromise() {
return {
then(res, rej) {
throw new Test262Error("Should not be evaluated");
// Throw a different error than Test262Error to avoid confusion about what is rejecting
throw new Error("Should not be evaluated");
},
};
}
Expand Down
4 changes: 2 additions & 2 deletions test/harness/asyncHelpers-throwsAsync-custom-typeerror.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ includes: [asyncHelpers.js]

var intrinsicTypeError = TypeError;

(async function () {
asyncTest(async function () {
function TypeError() {}
var caught = false;

Expand Down Expand Up @@ -69,4 +69,4 @@ var intrinsicTypeError = TypeError;
"assert.throwsAsync did not reject a collision of constructor names"
);
}
})().then($DONE, $DONE);
})
4 changes: 2 additions & 2 deletions test/harness/asyncHelpers-throwsAsync-custom.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ includes: [asyncHelpers.js]

function MyError() {}

(async function () {
asyncTest(async function () {
const p = assert.throwsAsync(MyError, function () {
return Promise.reject(new MyError());
});
assert(p instanceof Promise);
await p;
})().then($DONE, $DONE);
});
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@
// This code is governed by the BSD license found in the LICENSE file.
/*---
description: |
assert.throwsAsync returns a promise that rejects if funcOrThenable or the inner thenable synchronously throws.
assert.throwsAsync returns a promise that rejects if func or the inner thenable synchronously throws.
flags: [async]
includes: [asyncHelpers.js]
---*/

async function checkRejects(funcOrThenable) {
async function checkRejects(func) {
var caught = false;
const p = assert.throwsAsync(Test262Error, funcOrThenable);
const p = assert.throwsAsync(Test262Error, func);
assert(p instanceof Promise, "assert.throwsAsync should return a promise");
try {
await p;
Expand All @@ -18,34 +18,23 @@ async function checkRejects(funcOrThenable) {
assert.sameValue(
e.constructor,
Test262Error,
"throwsAsync should reject improper funcOrThenable with a Test262Error"
"throwsAsync should reject improper function with a Test262Error"
);
} finally {
assert(
caught,
"assert.throwsAsync did not reject improper funcOrThenable " +
funcOrThenable
"assert.throwsAsync did not reject improper function " + func
);
}
}

(async function () {
asyncTest(async function () {
await checkRejects(function () {
throw new Error();
});
await checkRejects(function () {
throw new Test262Error();
});
await checkRejects({
then: function () {
throw new Error();
},
});
await checkRejects({
then: function () {
throw new Test262Error();
},
});
await checkRejects(function () {
return {
then: function () {
Expand All @@ -60,4 +49,4 @@ async function checkRejects(funcOrThenable) {
},
};
});
})().then($DONE, $DONE);
});
4 changes: 2 additions & 2 deletions test/harness/asyncHelpers-throwsAsync-incorrect-ctor.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ flags: [async]
includes: [asyncHelpers.js]
---*/

(async function () {
asyncTest(async function () {
var caught = false;

const p = assert.throwsAsync(Error, function () {
Expand All @@ -32,4 +32,4 @@ includes: [asyncHelpers.js]
"assert.throwsAsync did not reject when a value with incorrect constructor was thrown"
);
}
})().then($DONE, $DONE);
});
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@
// This code is governed by the BSD license found in the LICENSE file.
/*---
description: |
assert.throwsAsync returns a promise that rejects if funcOrThenable is not a function returning a thenable or a thenable.
assert.throwsAsync calls $DONE with a rejecting value if func is not a function returning a thenable.
flags: [async]
includes: [asyncHelpers.js]
---*/

async function checkRejects(funcOrThenable) {
async function checkRejects(func) {
var caught = false;
const p = assert.throwsAsync(Test262Error, funcOrThenable);
const p = assert.throwsAsync(Test262Error, func);
assert(p instanceof Promise, "assert.throwsAsync should return a promise");
try {
await p;
Expand All @@ -18,28 +18,27 @@ async function checkRejects(funcOrThenable) {
assert.sameValue(
e.constructor,
Test262Error,
"throwsAsync should reject improper funcOrThenable with a Test262Error"
"throwsAsync should reject improper function with a Test262Error"
);
} finally {
assert(
caught,
"assert.throwsAsync did not reject improper funcOrThenable " +
funcOrThenable
"assert.throwsAsync did not reject improper function " + func
);
}
}

(async function () {
asyncTest(async function () {
await checkRejects(null);
await checkRejects({});
await checkRejects("string");
await checkRejects(10);
await checkRejects();
await checkRejects({ then: null });
await checkRejects({ then: {} });
await checkRejects({ then: "string" });
await checkRejects({ then: 10 });
await checkRejects({ then: undefined });
await checkRejects({
then: function (res, rej) {
res(true);
},
});
await checkRejects(function () {
return null;
});
Expand Down Expand Up @@ -68,4 +67,4 @@ async function checkRejects(funcOrThenable) {
await checkRejects(function () {
return { then: undefined };
});
})().then($DONE, $DONE);
});
4 changes: 2 additions & 2 deletions test/harness/asyncHelpers-throwsAsync-native.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ flags: [async]
includes: [asyncHelpers.js]
---*/

(async function () {
asyncTest(async function () {
var p = assert.throwsAsync(Error, async function () {
throw new Error();
});
Expand Down Expand Up @@ -44,4 +44,4 @@ includes: [asyncHelpers.js]
});
assert(p instanceof Promise);
await p;
})().then($DONE, $DONE);
});
4 changes: 2 additions & 2 deletions test/harness/asyncHelpers-throwsAsync-no-arg.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ flags: [async]
includes: [asyncHelpers.js]
---*/

(async function () {
asyncTest(async function () {
var caught = false;

const p = assert.throwsAsync();
Expand All @@ -29,4 +29,4 @@ includes: [asyncHelpers.js]
"assert.throwsAsync did not reject when invoked without arguments"
);
}
})().then($DONE, $DONE);
});
4 changes: 2 additions & 2 deletions test/harness/asyncHelpers-throwsAsync-no-error.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ flags: [async]
includes: [asyncHelpers.js]
---*/

(async function () {
asyncTest(async function () {
var caught = false;

const p = assert.throwsAsync(Error, async function () {});
Expand All @@ -29,4 +29,4 @@ includes: [asyncHelpers.js]
"assert.throwsAsync did not reject when the thenable did not reject"
);
}
})().then($DONE, $DONE);
});
4 changes: 2 additions & 2 deletions test/harness/asyncHelpers-throwsAsync-null.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ flags: [async]
includes: [asyncHelpers.js]
---*/

(async function () {
asyncTest(async function () {
var caught = false;

const p = assert.throwsAsync(Error, async function () {
Expand All @@ -31,4 +31,4 @@ includes: [asyncHelpers.js]
"assert.throwsAsync did not reject when null was thrown"
);
}
})().then($DONE, $DONE);
});
4 changes: 2 additions & 2 deletions test/harness/asyncHelpers-throwsAsync-primitive.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ flags: [async]
includes: [asyncHelpers.js]
---*/

(async function () {
asyncTest(async function () {
var caught = false;

const p = assert.throwsAsync(Error, async function () {
Expand All @@ -31,4 +31,4 @@ includes: [asyncHelpers.js]
"assert.throwsAsync did not reject when a primitive was thrown"
);
}
})().then($DONE, $DONE);
});
4 changes: 2 additions & 2 deletions test/harness/asyncHelpers-throwsAsync-resolved-error.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ flags: [async]
includes: [asyncHelpers.js]
---*/

(async function () {
asyncTest(async function () {
var caught = false;

const p = assert.throwsAsync(
Expand All @@ -32,4 +32,4 @@ includes: [asyncHelpers.js]
"assert.throwsAsync did not reject when the thenable resolved with an error"
);
}
})().then($DONE, $DONE);
});
6 changes: 3 additions & 3 deletions test/harness/asyncHelpers-throwsAsync-same-realm.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@
/*---
description: |
Thenables that reject with instances of the realm specified constructor function
satisfy the assertion, without cross realms collisions.
do not satisfy the assertion with cross realms collisions.
flags: [async]
includes: [asyncHelpers.js]
---*/

(async function () {
asyncTest(async function () {
var intrinsicTypeError = TypeError;
var caught = false;
var realmGlobal = $262.createRealm().global;
Expand All @@ -34,4 +34,4 @@ includes: [asyncHelpers.js]
"assert.throwsAsync did not reject when a different realm's error was thrown"
);
}
})().then($DONE, $DONE);
});
4 changes: 2 additions & 2 deletions test/harness/asyncHelpers-throwsAsync-single-arg.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ flags: [async]
includes: [asyncHelpers.js]
---*/

(async function () {
asyncTest(async function () {
var caught = false;
const p = assert.throwsAsync(function () {});
assert(p instanceof Promise);
Expand All @@ -28,4 +28,4 @@ includes: [asyncHelpers.js]
"assert.throwsAsync did not reject when invoked with a single argumemnt"
);
}
})().then($DONE, $DONE);
});

0 comments on commit eb44f67

Please sign in to comment.