Jasmine 3.2.0 breaks spyOn().and.returnValue(Promise.reject())

[kylekasky]

Expected Behavior

When trying to create a spy using spyOn(things, 'function).and.returnValue(Promise.reject()), I expect the function to have a spy.

Current Behavior

When trying to create a spy using spyOn(things, 'function).and.returnValue(Promise.reject()), the function does not change to a spy. It is the original function still. Doing so with a Promise.resolve() correctly makes a spy.

Logging out fetch.fetch within main shows the actual fetch function instead of the spy.
This behavior worked as intended until jasmine updated to 3.2.0;

import * as fetch from './fetch';
import Promise from 'bluebird';
export async function main(event, context, callback) {
    try {
        await Promise.map(things, (thing) => {
            return fetch.fetch(thing);
        });

        return callback();
    } catch (error) {
        console.error(error);
    }
}

import { main } from './thing';
import * as fetch from './fetch';
describe("main", function() {
    it('errors correctly', (done) => {
        const fetchSpy = spyOn(fetch, 'fetch').and.returnValue(Promise.reject({ message: 'failure' }));

        main({}, {}, () => {
            expect(fetchSpy).toHaveBeenCalled();
            done();
       });
    });
});

Context

Jasmine updating to 3.2.0 (via Package.json of ^3.0.0) broke our build machine of previously passing tests.

Your Environment

• Version used: 3.0.0, 3.1.0, 3.2.0 Jasmine
• Environment name and version (e.g. Chrome 39, node.js 5.4): Node 8.1.0
• Operating System and version (desktop or mobile): Mac Sierra & Alpine Linux build machine

[slackersoft]

Hmmm, it seems like this might be some kind of side effect from trying to capture unhandled rejections to report them with the spec, but I can get a reproduction case where the spy isn't actually installed. The extra stripped down example that I think should be equivalent to what you've got is (this should just strip out your fetch function being spied upon and bluebird):

const foo = { bar: function() {} };

async function main(callback) {
  try{
    await foo.bar();

    return callback();
  } catch(e) {
    console.error(e);
  }
}

describe("Apple", function() {
  it("mercury", function(done) {
    const thingSpy = spyOn(foo, 'bar').and.returnValue(Promise.reject({ message: 'failure' }));

    main(() => {
      expect(thingSpy).toHaveBeenCalled();
      done();
    });
  });
});

When I run this the console.error line prints out the error, but since the callback is never invoked because of the rejected Promise, Jasmine times out on the spec. If you only have the callback for your test code, you might take a look at the new async functionality in Jasmine and use async/await or return a Promise in your spec itself, which might just simplify your test code, though I wouldn't expect it to fix it.

Let me know if there is something crucial I stripped out in my sample spec or something else I've missed that might be contributing to the error.

[bobbyg603]

Any update here?

[slackersoft]

As I mentioned above, I haven't been able to reproduce this error, which makes it difficult to find a solution. If you can provide a simplified suite with all of the code that reproduces the issue, I'll be happy to take another look.

[bwknight877]

I think I have a repro, but using returnValues() using Jasmine 3.3.0 and nodejs 8.11.3:

file is saved as bar.spec.js

'use strict';

const bar = {
  test() {
    return Promise.resolve('Hello World');
  },
};

describe('bar', () => {
  fit('jasmine suite error', async () => {
    // multiple return values to test calls that may fail
    // a few times but succeed after a few retries
    spyOn(bar, 'test').and.returnValues(
      Promise.reject(new Error('bad')),
      Promise.reject(new Error('worse')),
      Promise.resolve('things are ok'),
    );

    try {
      await bar.test();
      fail();
    } catch (err) {
      expect(err.message).toBe('bad');
    }

    expect(bar.test).toHaveBeenCalled();
  });
});

running it, I get the following output:

~/source/jasmine-snafu (master #) $ npx jasmine *.spec.js
DEPRECATION: Setting throwOnExpectationFailure directly on Env is deprecated, please use the oneFailurePerSpec option in `configure`
DEPRECATION: Setting randomizeTests directly is deprecated, please use the random option in `configure`
Started
.


Suite error: bar
  Message:
    Error: worse
  Stack:
    Error: worse
        at UserContext.fit (/Users/azure.diamond/source/jasmine-snafu/bar.spec.js:15:22)
        at <Jasmine>
        at runCallback (timers.js:810:20)
        at tryOnImmediate (timers.js:768:5)
        at processImmediate [as _immediateCallback] (timers.js:745:5)

1 spec, 1 failure
Finished in 0.009 seconds

It seems like if you have returnValues() with multiple return values, and there are more than one Promise rejection in there, it will cause issues.

Based on the output though, the test succeeded, but the error was still thrown.

I have a few tests where I have retry logic around code that makes network calls, and I use returnValues() to return a few failed promises and then a successful one to make sure my retry logic will actually retry, but eventually succeed.

I updated another project to Jasmine 3.3.0 and had to fix this in a few tests of mine just last week. For whatever reason I can't create a simple repro using just .returnValue(Promise.reject(new Error('bad')) right now though, I'll update if I can revert my changes and condense it down to a simple repro

if I use bluebird instead of the default nodejs 8.x Promise library, I get a slightly different error:

~/source/jasmine-snafu (master *) $ npx jasmine
DEPRECATION: Setting throwOnExpectationFailure directly on Env is deprecated, please use the oneFailurePerSpec option in `configure`
DEPRECATION: Setting randomizeTests directly is deprecated, please use the random option in `configure`
Started
F

Failures:
1) bar jasmine suite error
  Message:
    Error: worse
  Stack:
    Error: worse
        at UserContext.it (/Users/azure.diamond/source/jasmine-snafu/bar.spec.js:17:22)
        at <Jasmine>
        at runCallback (timers.js:810:20)
        at tryOnImmediate (timers.js:768:5)
        at processImmediate [as _immediateCallback] (timers.js:745:5)
  Message:
    Failed: worse
  Stack:
    Error: worse
        at UserContext.it (/Users/azure.diamond/source/jasmine-snafu/bar.spec.js:17:22)
        at <Jasmine>
        at runCallback (timers.js:810:20)
        at tryOnImmediate (timers.js:768:5)
        at processImmediate [as _immediateCallback] (timers.js:745:5)
  Message:
    Error: <toHaveBeenCalled> : Expected a spy, but got Function.
    Usage: expect(<spyObj>).toHaveBeenCalled()
  Stack:
    Error: <toHaveBeenCalled> : Expected a spy, but got Function.
    Usage: expect(<spyObj>).toHaveBeenCalled()
        at <Jasmine>
        at UserContext.it (/Users/azure.diamond/source/jasmine-snafu/bar.spec.js:28:22)
        at <Jasmine>

1 spec, 1 failure
Finished in 0.011 seconds

[slackersoft]

My best guess here is that Node.js itself is detecting that the rejected promise exists and no code setup an error handler before it went out of scope and that is where the error is coming from. The fact that you're seeing different behavior when you use Bluebird seems to confirm some of this. I'm not sure what Jasmine can do to help with this.

A workaround you might try is to use callFake and only create the rejected Promise when the spy is actually invoked.

Hope this helps. Thanks for using Jasmine!

[elliot-nelson]

^ Agree on the workaround. Personally, I don't feel this is is a jasmine issue at all, it's a Bluebird issue.

The fix in general is to just never create rejected promises until you need them (from a jasmine perspective, that means always .callFake(() => Promise.reject()), never .returnValue()).

[sgravrock]

^ Agree on the workaround. Personally, I don't feel this is is a jasmine issue at all, it's a Bluebird issue.

The fix in general is to just never create rejected promises until you need them (from a jasmine perspective, that means always .callFake(() => Promise.reject()), never .returnValue()).

Agreed. I don't think Jasmine can allow .returnValue(Promise.reject()) without silencing all unhandled rejections, which isn't something we want to do. I'm closing this in favor of jasmine/jasmine.github.io#123 . If anyone is experiencing a flavor of this that can't be worked around by using .callFake, please open a new issue.