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

it, beforeEach, etc. no longer accept async functions #1320

Closed
pe8ter opened this issue Apr 25, 2017 · 4 comments
Closed

it, beforeEach, etc. no longer accept async functions #1320

pe8ter opened this issue Apr 25, 2017 · 4 comments

Comments

@pe8ter
Copy link

pe8ter commented Apr 25, 2017

Summary

Version 2.6.0 includes #1222 which forces functions as the argument for it, beforeEach, and the like. This introduces a breaking change because we can no longer pass async functions.

Example

describe('some suite', () => {
    it('is good', async (done) => {
        expect(await someAsyncLogic()).toBe(1);
        done();
    });
});

Expected result

Test should pass.

Actual result

Error: it expects a function argument; received [object AsyncFunction]

Environment

Jasmine: v2.6.0
Node.js: v7.9.0
Runner: gulp-jasmine v2.4.2

@slackersoft
Copy link
Member

Jasmine doesn't actually know how to handle async functions, so while it was not throwing an error before, it certainly wasn't waiting for the functions to complete correctly. For now, I think this is the desired behavior, but once something like #1270 has been merged, I'd be happy to talk about how to correctly support native async.

@pe8ter
Copy link
Author

pe8ter commented Apr 26, 2017

I'd argue for leaving the functionality as-is for now. It shouldn't matter what kind of function you throw at Jasmine because handling async behavior has always been up to the test writer anyway. Then introduce the breaking change once Jasmine has natural async behaviors built in.

I did find a workaround though. I updated my spec wrapping function to use the Promise API instead of async/await. Before 2.6.0:

function testAsync(specFunction) {
    return async (done) => {
        try {
            await specFunction();
            done();
        } catch (error) {
            done.fail(error);
        }
    };
}

After:

function testAsync(specFunction) {
    return (done) => {
        specFunction().then(() => {
            done();
        }).catch((error) => {
            done.fail(error);
        });
    };
}

This way, my specs don't have to change:

describe('some suite', () => {
    it('is good', testAsync(async () => {
        expect(await someAsyncLogic()).toBe(1);
    }));
});

@denzp
Copy link

denzp commented Apr 27, 2017

I made a workaround with "monkey patching" for my project. It waits for async function's returned promise to be resolved.

patchJasmineAsyncSpec('it');
patchJasmineAsyncSpec('fit');
patchJasmineAsyncSpec('beforeEach');
patchJasmineAsyncSpec('beforeAll');
patchJasmineAsyncSpec('afterEach');
patchJasmineAsyncSpec('afterAll');

function patchJasmineAsyncSpec(method) {
    const originalMethod = global[method];

    global[method] = function(name, callback) {
        if (callback && callback.constructor.name === 'AsyncFunction') {
            switch (callback.length) {
                case 0:
                    return originalMethod.call(this, name, function(done) { callback.call(this).then(done, done.fail); });

                default:
                    throw new Error('Async function should not have "done" callback!');
            }
        }

        originalMethod.call(this, ...arguments);
    }
}

Edit... Here is a correct version that works with async beforeEach, beforeAll, etc.

function patchJasmineAsyncSpec(method) {
    const originalMethod = global[method];

    global[method] = function(...args) {
        args = args.map(argument => {
            if (argument && argument.constructor.name === 'AsyncFunction') {
                switch (argument.length) {
                    case 0:
                        return function(done) { argument.call(this).then(done, done.fail); };

                    default:
                        throw new Error('Async function should not have "done" callback!');
                }
            }

            return argument;
        });

        originalMethod.call(this, ...args);
    }
}

@niieani
Copy link

niieani commented Apr 29, 2017

@denzp Awesome. Thanks for this!

If you use babel/typescript to transpile await/async, you can also use this version to patch jasmine to support returning Promises:

patchJasmineAsyncSpec('it');
patchJasmineAsyncSpec('fit');
patchJasmineAsyncSpec('beforeEach');
patchJasmineAsyncSpec('beforeAll');
patchJasmineAsyncSpec('afterEach');
patchJasmineAsyncSpec('afterAll');

function patchJasmineAsyncSpec(method) {
  const originalMethod = global[method];

  global[method] = function (...args) {
    args = args.map(argument => {
      if (argument) {
        if (argument.constructor.name === 'AsyncFunction') {
          switch (argument.length) {
            case 0:
              return function (done) { argument.call(this).then(done, done.fail); };
            default:
              throw new Error('Async function should not have a "done" callback!');
          }
        } else if (argument.constructor.name === 'Function' && !argument.length) {
          return function (done) {
            try {
              const maybePromise = argument.call(this)
              if (maybePromise.then) {
                maybePromise.then(done, done.fail);
              } else {
                done();
              }
            } catch (e) {
              done(e);
            }
          }
        }
      }

      return argument;
    });

    originalMethod.call(this, ...args);
  }
}

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

No branches or pull requests

4 participants