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

Asserting error thrown no longer works as of 2.11 #15013

Closed
ykaragol opened this issue Mar 14, 2017 · 15 comments
Closed

Asserting error thrown no longer works as of 2.11 #15013

ykaragol opened this issue Mar 14, 2017 · 15 comments

Comments

@ykaragol
Copy link

ykaragol commented Mar 14, 2017

The following test, no longer run after we start to use 2.11.1.

test('it throws error my-custom-input is called', function(assert) {
    assert.throws(() => {
        this.render(hbs`{{my-custom-input}}`);
    }, /my-custom-input component error/, 'Error must have been thrown');
});

I saw mocha users had faced with nearly the same problems: emberjs/ember-mocha#141

SO Question: http://stackoverflow.com/questions/42781085/ember-render-hbs-swallowing-thrown-error
Slack Archive: https://embercommunity.slack.com/archives/-testing/p1489479464001332

Repro
https://ember-twiddle.com/47c4cfbb571ac3fa83ab912f605ebc6a?fileTreeShown=false&openFiles=tests.integration.components.i-throw-test.js%2C

@Turbo87
Copy link
Member

Turbo87 commented Mar 14, 2017

/cc @rwjblue @krisselden @workmanw

@ghost
Copy link

ghost commented Mar 14, 2017

I am seeing identical errors with should.js and expect style assertions, also using mocha. Though, I only see this when bumping from 2.11.2 to 2.11.3.
similar pattern failing using expect and should.js:

(function() {
	this.render(hbs`{{async-image}}`);
}).should.throw('need a source');

@pixelhandler
Copy link
Contributor

Is this something that can be replicated using ember-twiddle?

@ghost
Copy link

ghost commented Mar 17, 2017

@pixelhandler i'm struggling getting an ember-mocha twiddle set up, any tips / examples?
started here: https://ember-twiddle.com/49a310682db5d8433dff5ef98ab75211?

@mupkoo
Copy link

mupkoo commented Mar 17, 2017

I tried to reproduce it without any success. Maybe something related to the Ember versions in the Twiddle?

https://ember-twiddle.com/8932ace19517f83ef12693068381b6a3?openFiles=twiddle.json%2C

@ykaragol
Copy link
Author

As one of the Stackoverflow comment says they could not reproduced it in twiddle.
But checkout this repo: https://github.com/feanor07/ember-component-init-error-swallowed

@nickiaconis
Copy link
Contributor

nickiaconis commented Mar 22, 2017

I've reproduced this in a Twiddle. The key to reproduction is to run any acceptance test before running the test that asserts the thrown error. Running the acceptance test populates Ember.Test.adapter, and it is that adapter that handles the error before assert.throws can receive it.

https://ember-twiddle.com/47c4cfbb571ac3fa83ab912f605ebc6a?fileTreeShown=false&openFiles=tests.integration.components.i-throw-test.js%2C

cc @pixelhandler

Robdel12 added a commit to adopted-ember-addons/emberx-select that referenced this issue Mar 24, 2017
According to this
ticket (emberjs/ember.js#15013) and this
ticket (emberjs/ember-mocha#141) ember 2.11
broke testing exceptions.
Robdel12 added a commit to adopted-ember-addons/emberx-select that referenced this issue Mar 24, 2017
* Upgrade ember-cli 2.11.0

* Upgrade to ember-cli 2.12.1

* Update mocha tests

* ember#release seems to be borked.

* Skip test related to testing thrown errors

According to this
ticket (emberjs/ember.js#15013) and this
ticket (emberjs/ember-mocha#141) ember 2.11
broke testing exceptions.
@nickiaconis
Copy link
Contributor

Here's a work-around for QUnit:

import Ember from 'ember';
import QUnit from 'qunit';
import { QUnitAdapter } from 'ember-qunit';

// Work-around for https://github.com/emberjs/ember.js/issues/15013
const originalAssertThrows = QUnit.assert.throws;
const rethrowAdapter = QUnitAdapter.extend({
  exception(error) {
    throw error;
  },
}).create();
QUnit.assert.throws = () => {
  const originalAdapter = Ember.Test.adapter;
  Ember.Test.adapter = rethrowAdapter;

  originalAssertThrows.apply(this, arguments);

  Ember.Test.adapter = originalAdapter;
};

bmeurant pushed a commit to bmeurant/ember-array-contains-helper that referenced this issue Apr 9, 2017
bmeurant pushed a commit to bmeurant/ember-array-contains-helper that referenced this issue Apr 9, 2017
bmeurant pushed a commit to bmeurant/ember-array-contains-helper that referenced this issue Apr 9, 2017
Asserting error thrown no longer works as of ember 2.11
see (emberjs/ember.js#15013) and (emberjs/ember-mocha#141)
bmeurant pushed a commit to bmeurant/ember-array-contains-helper that referenced this issue Apr 9, 2017
Asserting error thrown no longer works as of ember 2.11
see (emberjs/ember.js#15013) and (emberjs/ember-mocha#141)
@ghost
Copy link

ghost commented Apr 19, 2017

I've narrowed down the problem to the following change: a90fa06#diff-33ff1529a427727ffe2425f145375540L24. Reverting the change back to getOnerror() produces the expected assertions in my project. I'm going to keep tinkering if there is a way to exempt test assertions from dispatching the entire events.
that said workmanw might have the better/quicker solution here.

theory as why this happens

Chai's underlying assertThrows (https://github.com/chaijs/chai/blob/526f88585d32220d3777053530ab3182af7bf5d0/lib/chai/core/assertions.js#L2430-L2435) check expects the invoked function to execute synchronously, but the refactor to dispatchError executes asynchronously so chai's catch doesn't capture the error.
/cc @workmanw

tzellman added a commit to tzellman/ember-reactive-helpers that referenced this issue May 3, 2017
Note that I cannot update to ember 2.12 just yet until there is a resolution for emberjs/ember.js#15013
@tzellman
Copy link

tzellman commented May 3, 2017

Just came across this same issue while attempting to update an addon from ember/cli 2.9. Has anyone found a decent workaround for ember-cli-chai usage?

tzellman added a commit to tzellman/ember-reactive-helpers that referenced this issue May 3, 2017
For now, I am skipping the three tests that assert that errors are thrown, until I can find a resolution for emberjs/ember.js#15013
Additionally, I fixed the _lookupFactory deprecation in favor of factoryFor
Also, removed a few jshint pragmas
@alvincrespo
Copy link
Contributor

I'm currently working on our ember-link-with-follower update and running across this issue in our tests: echobind/ember-links-with-follower#191. We're using ember-cli-mocha.

@pixelhandler
Copy link
Contributor

@ykaragol I'm curious if this is still an issue in 2.16.x ?

@ykaragol
Copy link
Author

ykaragol commented Nov 6, 2017

@pixelhandler I didn't test it. I'd just use the workaround shown in the StackOverflow Question.

@rwjblue
Copy link
Member

rwjblue commented Nov 6, 2017

The ember-qunit-assert-helpers addon was specifically created to handle this scenario...

@rwjblue
Copy link
Member

rwjblue commented Nov 28, 2017

This was fixed by #15871, which essentially reverts #14898 and adds a bunch of tests.

@rwjblue rwjblue closed this as completed Nov 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants