Allowing rejected promises in tests

[marcoow]

I'm writing tests for Ember Simple Auth and want to test the case that the authenticator rejects authentication. Therefor I stub its authenticate method to return a rejected promise:

sinon.stub(authenticator, 'authenticate').returns(Ember.RSVP.reject('error'));

The problem now ist that this immediately causes RSVP.onerrorDefault to run which fails the test: Test.adapter.exception(error);.

I don't see a clean way to disable that behavior so that it allows this specific promise rejection. A working solution I came up with is

var originalPromiseError;

beforeEach(function() {
  originalPromiseError = Ember.RSVP.Promise.prototype._onerror;
  Ember.RSVP.Promise.prototype._onerror = Ember.K;
  sinon.stub(authenticator, 'authenticate').returns(Ember.RSVP.reject('error'));
});

afterEach(function() {
  Ember.RSVP.Promise.prototype._onerror = originalPromiseError;
});

but that's obviously not sth. I want to be doing as it uses private API.

I think it would be great to have some way of using unhandled promise rejections in tests. While that would most likely not be of use for applications, I assume that more addons than just ESA might have a need for sth. like this.

[marcoow]

This only happens when I do the stubbing in beforeEach (I'm using mocha), not in an it. I assume that that works as the rejection is handled inside of the it but not in the beforeEach?

[stefanpenner]

seems dubious to allow rejections during tests

[marcoow]

Yeah, understand that. But isn't the use case actually valid?

[hwhelchel]

I believe I'm running up against this in my acceptance tests for ESA and ESA devise. What I have is

describe('with invalid login', function() {
  beforeEach(function() {
    this.options.status = 401;

    TestHelper.stubEndpointForHttpRequest('/accounts/sign_in', { error: 'invalid_grant' }, this.options);
    visit('/accounts/sign_in');
    fillIn('#email', this.account.email);
    fillIn('#password', this.account.password);
    click('input[type=submit]'); // blows up here when authenticate promise rejects
  });

  it('should not validate the session', function() {
    andThen(() => {
      expect(currentSession().isAuthenticated).to.equal(false);
      expect(currentSession().get('content.secure')).not.to.include.keys(...Object.keys(this.resp));
    });
  });
});

Is there an alternative way of testing this user flow?

[jamesarosen]

@stefanpenner

seems dubious to allow rejections during tests

Seems perfectly normal to me. How else would I test that I show an appropriate error message when I get a 422 response from some API?

Also: this behavior is not just bad, it's also inconsistent.

This doesn't break the test:

new Ember.RSVP.Promise(function(resolve, reject) {
  reject();
});

But this does:

new Ember.RSVP.Promise(function(resolve, reject) {
  reject("anything");
});

because in the first case Test.adapter.exception(error); is Test.adapter.exception(undefined); and the test adapter ignores it.

[jamesarosen]

Note that this is intimately related to ember-cli/rfcs#19, which proposes switching ember-cli's default AJAX adapter from ic-ajax to a fetch wrapper. Unlike jQuery.ajax, fetch (per spec), resolves successfully with an error object for 4xx and 5xx.

It may be the case that the long-term path for Ember is don't use reject and .catch(fn) for control flow. But that's not what apps expect right now.

[stefanpenner]

@jamesarosen I should have clarified. Unhandled Rejections with error objects are dubious to allow.

Handle rejections or unhandled without a reason that is instanceof error. Are fine, and should already not be asserted

[jamesarosen]

Somehow I think you and I are heading towards the same conclusion, yet I don't quite understand what you said ;)

Which of the following are OK in your mind?

// reject with undefined, no catch:
new Ember.RSVP.Promise(function(resolve, reject) {
  reject();
});

// reject with undefined, catch:
new Ember.RSVP.Promise(function(resolve, reject) {
  reject();
}).catch(Ember.K);

// reject with string, no catch:
new Ember.RSVP.Promise(function(resolve, reject) {
  reject("tsk tsk!");
});

// reject with string, catch:
new Ember.RSVP.Promise(function(resolve, reject) {
  reject("tsk tsk!");
}).catch(Ember.K);

// reject with object, no catch:
new Ember.RSVP.Promise(function(resolve, reject) {
  reject({ any: 'old', object: 'literal' });
});

// reject with object, catch:
new Ember.RSVP.Promise(function(resolve, reject) {
  reject({ any: 'old', object: 'literal' });
}).catch(Ember.K);

// reject with Error, no catch:
new Ember.RSVP.Promise(function(resolve, reject) {
  reject(new Error("tsk tsk!"));
});

// reject with Error, catch:
new Ember.RSVP.Promise(function(resolve, reject) {
  reject(new Error("tsk tsk!"));
}).catch(Ember.K);

[stefanpenner]

All except for the unhandled rejection that rejects with the error. ( sorry on mobile or I would have cited the one)

[jamesarosen]

All except for the unhandled rejection that rejects with the error

This I can get on board with :)

[stefanpenner]

@jamesarosen yup I thought so. And that is also the current behavior. It also handles more rejection scenarios. As long as they are handled in the same turn of the actions queue flush they will be good to go.

I can likely expand that further yet, as we recently added some extra hooks to the run-loop.

Anyways, I believe the current state is actually what one wants.

If what I described does not happen, we must consider it a regression and fix .

[twokul]

@marcoow I think you can work around it by Ember.Test.adapter = null; in beforeEach or before (depends if it's qunit or mocha)

[jamesarosen]

And that is also the current behavior.

Then why is the following code breaking?

function failToDoX() {
  return new Ember.RSVP.Promise(function(resolve, reject) {
    Ember.run(null, reject, { an: 'error' });
  });
}

test('it catches', function(assert) {
  var caught = false;

  failToDoX()
  .catch(function(e) { caught = e; });

  assert.ok(caught != null);
});

I get to the catch, but the test still fails. Is it because failToDoX does its own run-loop wrapping?

[twokul]

@jamesarosen I might be wrong but I think it depends on how Ember.Test.adapter is setup up.

RSVP. onerrorDefault will check if there's an adapter set up and depending on the result either throw an error (which causes catch to be called w/o side effects) or it will print an error in the console and add an ok(false) assertion.

So the test above should work in isolation but it might not work when running whole test suite with acceptance/integration/unit tests.

[jamesarosen]

@twokul yeah, but that doesn't jibe with what @stefanpenner is saying:

All except for the unhandled rejection that rejects with the error [should be OK]

and

And that is also the current behavior. It also handles more rejection scenarios.

What worries me is

As long as they are handled in the same turn of the actions queue flush they will be good to go.

That suggests to me that if something does its own run-loop-wrapping, I can't .catch to prevent the promise failure from bubbling.

[stefanpenner]

Ember.run(null, reject, { an: 'error' });

2 things:

the run forces the flush early that being said, I'm not sure why a non-error object is causing the assertion to throw, that appears like a bug.

[stefanpenner]

these lines dont quite look correct https://github.com/emberjs/ember.js/blob/master/packages/ember-runtime/lib/ext/rsvp.js#L60-L67

[stefanpenner]

Ultimately if something doesn't jive with what i said, it is very likely a bug/omission/regression

[stefanpenner]

@marcoow I think you can work around it by Ember.Test.adapter = null; in beforeEach or before (depends if it's qunit or mocha)

this really isn't advised, be aware of the :footprints: :gun:

[marcoow]

@stefanpenner: yeah, that might hide the problem but of course isn't actually a solution.

IIRC, I ran into this: https://github.com/emberjs/ember.js/blob/master/packages/ember-runtime/lib/ext/rsvp.js#L61 and that actually makes the test fail regardless of whether the rejection is handled in the test or not.

[stefanpenner]

I have enumerated an added tests to the current behavior: https://github.com/emberjs/ember.js/pull/11903/files

[stefanpenner]

@marcoow https://github.com/emberjs/ember.js/blob/master/packages/ember-runtime/lib/ext/rsvp.js#L61 should be fixed with #11903 Turns out we were using a quite old version of RSVP and some more recent refactoring scenario solved the problem. This should correctly align it with my above statements, and also add the appropriate test cases.

https://github.com/tildeio/rsvp.js/blob/master/lib/rsvp/promise.js#L177-L181

[marcoow]

@stefanpenner great, looks good!

[jamesarosen]

All of a sudden I'm seeing this on Ember v1.13.11, which uses RSVP v3.0.14.

The relevant code looks like

function someMethod() {
  if (someCondition) {
    return RSVP.reject({ type: 'error', body: 'someCondition failed' });
  }
}

and the relevant test:

test('when someCondition is false', function(assert) {
  return theObject.someMethod()
  .catch((error) => {
    assert.equal(error.type, 'error', 'rejects with an error');
  })
});

When I run the test in isolation, it passes. When I run the full test suite, I get

not ok 778 PhantomJS 2.1 - Integration: ApiKeys: apiKeys.load rejects when sudo is not enabled
    ---
        actual: >
            false
        expected: >
            true
        stack: >
            http://localhost:7357/assets/test-support.js:3879:17
            ...
        message: >
            {type: error, body: someCondition failed}

[stefanpenner]

@jamesarosen that doesn't seem correct and the catch handler is attached sync. Can you provide a quick reproduction?

[jamesarosen]

and the catch handler is attached sync

Oh! Maybe it's coming from an earlier test. Like one I'm not attaching a catch handler to because I expect it to pass.

[jamesarosen]

Ah, one nuance. It's actually

return Ember.run(theObject, 'someMethod')
.catch((error) => {
  assert.equal(error.type, 'error', 'rejects with an error');
});

[jamesarosen]

The minimal test case on Ember 1.13.11 is

test('something', function(assert) {
  return Ember.run(function() {
    return Ember.RSVP.reject({ type: 'error', body: 'any old error' });
  })
  .catch(() => {
    assert.equal(1, 1, 'go to the 1==1 test');
  });
});

This fails with

not ok 1 PhantomJS 2.1 - Unit: CappedArray: anything
    ---
        actual: >
            null
        stack: >
            onerrorDefault@http://localhost:7357/assets/vendor.js:44114:22
            trigger@http://localhost:7357/assets/vendor.js:68380:19
            _onerror@http://localhost:7357/assets/vendor.js:69346:29
            publishRejection@http://localhost:7357/assets/vendor.js:67653:23
            http://localhost:7357/assets/vendor.js:44080:15
            invoke@http://localhost:7357/assets/vendor.js:11600:20
            flush@http://localhost:7357/assets/vendor.js:11664:17
            flush@http://localhost:7357/assets/vendor.js:11465:22
            end@http://localhost:7357/assets/vendor.js:10754:30
            run@http://localhost:7357/assets/vendor.js:10876:21
            run@http://localhost:7357/assets/vendor.js:31717:32
            http://localhost:7357/assets/tests.js:26216:33
            runTest@http://localhost:7357/assets/test-support.js:2528:32
            run@http://localhost:7357/assets/test-support.js:2513:11
            http://localhost:7357/assets/test-support.js:2655:14
            process@http://localhost:7357/assets/test-support.js:2314:24
            begin@http://localhost:7357/assets/test-support.js:2296:9
            http://localhost:7357/assets/test-support.js:2356:9
        message: >
            Died on test #1 http://localhost:7357/assets/tests.js:26215:19
            exports@http://localhost:7357/assets/vendor.js:92:39
            build@http://localhost:7357/assets/vendor.js:142:17
            findModule@http://localhost:7357/assets/vendor.js:190:14
            requireModule@http://localhost:7357/assets/vendor.js:177:22
            require@http://localhost:7357/assets/test-loader.js:60:16
            loadModules@http://localhost:7357/assets/test-loader.js:51:25
            load@http://localhost:7357/assets/test-loader.js:82:35
            http://localhost:7357/assets/test-support.js:6209:20: [object Object]
        Log: |
    ...

but I expect it to fail with

actual: >
            1
        expected: >
            2
        stack: >
            ...
        message: >
            1 == 2

[jamesarosen]

Or, more minimally,

test('anything', function(assert) {
  Ember.run(() => {
    Ember.RSVP.reject({ type: 'error', body: 'bad foobar' });
  });

  assert.equal(1, 2, '1 == 2');
});

which fails with

    ---
        actual: >
            null
        stack: >
            onerrorDefault@http://localhost:7357/assets/vendor.js:44114:22
            trigger@http://localhost:7357/assets/vendor.js:68380:19
            _onerror@http://localhost:7357/assets/vendor.js:69346:29
            publishRejection@http://localhost:7357/assets/vendor.js:67653:23
            http://localhost:7357/assets/vendor.js:44080:15
            invoke@http://localhost:7357/assets/vendor.js:11600:20
            flush@http://localhost:7357/assets/vendor.js:11664:17
            flush@http://localhost:7357/assets/vendor.js:11465:22
            end@http://localhost:7357/assets/vendor.js:10754:30
            run@http://localhost:7357/assets/vendor.js:10876:21
            run@http://localhost:7357/assets/vendor.js:31717:32
            http://localhost:7357/assets/tests.js:26216:26
            runTest@http://localhost:7357/assets/test-support.js:2528:32
            run@http://localhost:7357/assets/test-support.js:2513:11
            http://localhost:7357/assets/test-support.js:2655:14
            process@http://localhost:7357/assets/test-support.js:2314:24
            begin@http://localhost:7357/assets/test-support.js:2296:9
            http://localhost:7357/assets/test-support.js:2356:9
        message: >
            Died on test #1 http://localhost:7357/assets/tests.js:26215:19
            exports@http://localhost:7357/assets/vendor.js:92:39
            build@http://localhost:7357/assets/vendor.js:142:17
            findModule@http://localhost:7357/assets/vendor.js:190:14
            requireModule@http://localhost:7357/assets/vendor.js:177:22
            require@http://localhost:7357/assets/test-loader.js:60:16
            loadModules@http://localhost:7357/assets/test-loader.js:51:25
            load@http://localhost:7357/assets/test-loader.js:82:35
            http://localhost:7357/assets/test-support.js:6209:20: [object Object]
        Log: |

[rwjblue]

Have you tested this on a recent Ember version? A number of issues have been resolved in the last year that do not exist in 1.13, it is possible that this is already resolved.

[jamesarosen]

We're a log way from being able to upgrade beyond 1.13. I'll test it in some more recent Embers and let you know. Maybe that will help us find where it was fixed and back port that.

[stefanpenner]

@jamesarosen you describe expected behavior, this does not look like a bug.

[jamesarosen]

How would you test that something that requires set (and this the run-loop) rejects without the test failing? For example, how would you test both paths of the following method?

export default Ember.Session.extend({
  failedAttempts: 0,
  signedIn: false,

  signIn(username, password) {
    return ajax({ url: '/sign-in', type: 'post', data: { username, password })
    .then(() => {
      this.set('signedIn', true);
    })
    .catch(() => {
      this.incrementProperty('failedAttempts');
      return Ember.RSVP.reject({ type: 'error', body: 'Invalid username or password' });
    });
  }
}

[stefanpenner]

The rule of thumb is as follows: If you have a rejection and if by the end of the current run-loop it is not handled, we assume it may never be handled and fail your test.

So if a handler is attached before the end of the run-loop, all will be fine.

In your example, whoever the consumer of singIn is must handle the rejection before the end of the current run-loop.

[jamesarosen]

Ah! So this should work:

test('foo', function(assert) {
  Ember.run(() => {
    thing.signIn('username', 'bad password')
    .catch((err) => {
      assert.equal(err.body, 'Invalid username or password');
    });
  });
}):

[stefanpenner]

@jamesarosen ya, now i understand this seems WAT. But it is the way it is after much consideration. It is a tricky balance we must strike. If others have better ideas, I'll listen but just be warned its a tricky balancing act...

[jamesarosen]

OK. This test helper to wrap everything in an Ember.run. That way, test authors don't reach for Ember.run inside their tests and get bitten by rejected promises.

import Ember from 'ember';
import { test } from 'ember-qunit';
const { run } = Ember;

export default function testInRunLoop(name, callback) {
  test(name, function(assert) {
    const test = this;
    return run(test, callback, assert);
  });
}

[stefanpenner]

@jamesarosen I would consider that test helper an anti-pattern, but the solution would be to simply handle the result of the callback before returning to run itself. So it is considered handled before the turn completes.

[jamesarosen]

I would consider that test helper an anti-pattern

I have no doubt you would. But the only other option is to wrap each test in Ember.run(() => {...}) since the thing that catches the rejection is the test. It's not some other client in the app. The test is asserting that the method returned a rejection. And I need the run because there are set calls. Without it, I get the "You turned on testing mode, which disabled the auto-run."

If I seem frustrated, it's because I am. I appreciate your work and your help on this issue, but everything I suggest gets dismissed as "not what the core team wants you to do."

I'm absolutely open to other ideas, though! If you have a suggestion on how to test -- at an integration or unit level -- that a method returns a rejected promise under certain conditions and has set behavior under those conditions, I'm all ears!

[stefanpenner]

"not what the core team wants you to do."

This often correlates to, don't cause yourself extra pain/frustration. The suggestions are not arbitrary.

I'm absolutely open to other ideas, though! If you have a suggestion on how to test -- at an integration or unit level -- that a method returns a rejected promise under certain conditions and has set behavior under those conditions, I'm all ears!

can you provide a jsbin, or a demo app with the problem and i will gladly do so.

[jamesarosen]

Thanks, @stefanpenner. I'll keep digging and see what I can turn up.

[jamesarosen]

@stefanpenner pointed me in the right direction. Ember.RSVP will automatically enqueue run-loops without Ember.run and it won't trigger the auto-run error in test mode. That means I can just do

test('foo', function(assert) {
  return thing.signIn('username', 'bad password')
  .catch((err) => {
    assert.equal(err.body, 'Invalid username or password');
  });
}):

and it works just fine :)

[stefanpenner]

@jamesarosen we have been slowly massaging all these to continue to remove traps/pain points, its very possible your recall a previous state where this was not handled transparently. I suspect you will continue to see improvement in this domain.

[jamesarosen]

Given that ember-data v1.x calls Ember.run(null, reject, someError), do you have any suggestions on how to best turn off these erroneous test failures? Is it as simple as overriding ajax to not wrap in run?

[toranb]

I'm not here to debate the "right or wrong" of testing exceptions but I did want to offer a workaround for those like me who value testing 500 errors in acceptance tests (whether you are using ember-data or something else that wraps RSVP). Full credit for this goes to my good friend @williamsbdev who found the least hacky/but most functional way to get this working :)

var application, originalLoggerError, originalTestAdapterException;

module('Acceptance to tdd error handling in your ember app', {
    beforeEach() {
        application = startApp();
        originalLoggerError = Ember.Logger.error;
        originalTestAdapterException = Ember.Test.adapter.exception;
        Ember.Logger.error = function() {};
        Ember.Test.adapter.exception = function() {};
    },
    afterEach() {
        Ember.Logger.error = originalLoggerError;
        Ember.Test.adapter.exception = originalTestAdapterException;
        Ember.run(application, 'destroy');
    }
});

test('some test that blows up when I try to tdd and a 500 makes RSVP reject', (assert) => {
    server.post('/api/foo/1', (db, request) => {
        //return a 500 with ember-cli-mirage/or whatever ajax stub library you choose
    });
    visit('/foo');
    fillIn('#my-input', 999);
    click('#my-btn');
    andThen(() => {
        //assert your modal shows up/or whatever biz thing happened
    });
});

[williamsbdev]

@toranb - Thank you for calling attention to this.

I would like to give some background. One of the applications that we were working on had great validations before the request was made so it would not get a 400. Instead of handling all the errors with each ajax request, we wanted to catch all the errors across the application. So we setup something like this:

Ember.RSVP.on("error", function() {
    // handle 401
    // handle 403
    // handle 5XX errors
    // handle other errors
});

The problem then came when we wanted to test. This is why the monkey patching (done above in the comment by @toranb) on Ember.Logger.error and Ember.Test.adapter.exception were done. I was not terribly fond of what I did but it gave me some confidence that the application would respond in the desired manner.

I completely understand what @stefanpenner was saying about handling the error by the end of the run loop. When running the tests, you don't know if the error has been handled and the test framework is going to let the user know that something is broken. This is great. We just took advantage of the Ember.RSVP.on("error" which allowed it to handle the error and had to trick the test into allowing it.

[sukima]

FYI this is especially troublesome when attempting to use async/await The wrapping in a run loop is not something you can accomplish with async functions because run is not asynchronous. You can not express this using async/await, co-routines (like ember-co) or ember-concurrency tasks in a unit test:

test('it really has come to this', function (assert) {
  return run(() => {
    return RSVP.then(() => {
      assert.ok(false, 'expected promise to be rejected');
    }).catch(error => {
      assert.ok(true, 'expected promise to be rejected');
    });
  });
});

I challenge anyone who can show how to express a try { await ... } catch { } inside an Ember.run(). Good luck!

It is sad this is the state of affairs. If only there was a way to pause a run loop do you thing and the resume the run loop.

[dmuneras]

I think this post from EmberMap could be very helpful for people that end up in this conversation:

https://embermap.com/notes/86-testing-mirage-errors-in-your-routes

[lolmaus]

@dmuneras The article by @samselikoff describes my case precisely:

GIVEN that my server will error in response to XYZ network request WHEN my app makes a request to XYZ THEN I expect my app to behave in this way

Unfortunately, the article only covers suppressing console logging. It does not address the original issue: preventing an acceptance test from failing when an Ember Data request rejects and is unhandled.

We intercept the error with a decorator that shows a flash message to the user. But we re-throw the error, so that it's caught by an error reporter such as Sentry.

The solution outlined above, overriding Ember.Test.adapter.exception, does not work because of this:

// Ember 2.17 and higher do not require the test adapter to have an `exception`
// method When `exception` is not present, the unhandled rejection is
// automatically re-thrown and will therefore hit QUnit's own global error
// handler (therefore appropriately causing test failure)

https://github.com/emberjs/ember-qunit/blob/5cb60f696c24667f0cc2ce73e575d9f2b1d1a62c/addon-test-support/ember-qunit/adapter.js#L60-L75

Any ideas?

PS Yes, I know that we could have our flash message decorator report to Sentry directly, so that we don't need to re-throw the error. But that's not its responsibility! And I don't want to sprinkle the app with Sentry reportings, instead of having it intercept all errors globally.

[amk221]

@lolmaus For that exact scenario I do this:

// Inside a test
this.errorHandlingService.squelch(error => {
  return error instanceof InternalServerError;
});

...where the error handling service looks like this