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

Support async functions as callback in pm.test #919

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

meDavidNS
Copy link

@meDavidNS meDavidNS commented May 10, 2023

Async functions return a promise. Currently with pm.test('description', async function () {}) will mark the test as successful and completed, even if an exception is thrown.

Async functions return a promise. Currently with pm.test('description', async function () {}) will mark the test as successfull and completed, even if an exception is thrown.
@meDavidNS
Copy link
Author

The PR would still require some tests, but I would first welcome some feedback on the proposed change before tuning the PR.

@@ -104,12 +104,16 @@ module.exports = function (pm, onAssertionComplete) {
}
// if the assertion function does not expect a callback, we synchronously execute the same
else {
try { assert(); }
try {
Promise.resolve(assert())
Copy link
Member

Choose a reason for hiding this comment

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

The vast majority of tests aren't going to be async functions, and we'll likely hit a small performance penalty on running all test functions through Promise resolution. Wondering if we could do something like...

try {
  const maybePromise = assert();
  if (maybePromise instanceof Promise) {
    Promise.resolve(maybePromise)
      .catch(e => markAssertionAsFailure(assertionData, e));
  }
} catch (e) {
  markAssertionAsFailure(assertionData, e);
}

onAssertionComplete(assertionData);

Alternatively, we could also check if the function's constructor name is AsyncFunction and split logic based on that without having to execute the function first.

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.

2 participants