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

render does not reject when rendering fails #310

Open
ef4 opened this issue Feb 5, 2018 · 15 comments · May be fixed by #1194
Open

render does not reject when rendering fails #310

ef4 opened this issue Feb 5, 2018 · 15 comments · May be fixed by #1194

Comments

@ef4
Copy link
Contributor

ef4 commented Feb 5, 2018

I would have expected this test to pass, but it fails with "expected not to get here" in addition to an out-of-band qunit failure for the actual Compiler Error: not-a-component is not a helper.

test('render can throw', async function(assert) {
  try {
    await render(hbs`{{not-a-component x="y"}}`);
  } catch (err) {
    assert.ok(/not-a-component is not a helper/.test(err.message), "expected to see failure");
  }
  throw new Error("expected not to get here");
});
@rwjblue
Copy link
Member

rwjblue commented Feb 5, 2018

This is ultimately due to how rendering is deferred in the render queue.

There are work arounds / solutions, I’ll try to list them so we can discuss the best path forward.

@ef4
Copy link
Contributor Author

ef4 commented Feb 5, 2018

Yeah, I know the scheduling isn't synchronous, I was just a little surprised because we're already waiting here for everything to finish, so most of the plumbing for getting the state threaded back to us must already be present.

@cibernox
Copy link
Contributor

I'm also trying to find a reliable way of testing errors/assertions during the render. What are the possible workarounds for this? (Or reliable fix for that matter)

@kellyselden
Copy link
Member

This appears to be a bit stale, but I would love some workarounds as well.

@kellyselden
Copy link
Member

kellyselden commented May 8, 2018

With the help of @rwjblue's talk https://www.youtube.com/watch?v=PwQAj5UU9ng, I came up with a decent workaround.

  let onerror;

  hooks.beforeEach(function() {
    // `sinon.stub(Ember, 'onerror')` won't work because onerror
    // has a setter, it's not the real reference called
    onerror = Ember.onerror;
  });

  hooks.afterEach(function() {
    Ember.onerror = onerror;
  });

  test('it fails when passed a number', async function(assert) {
    let stub = sinon.stub();
    Ember.onerror = stub;

    await render(hbs`{{zip-filter 98101}}`);

    assert.ok(stub.withArgs(sinon.match({
      message: 'Assertion Failed: zip-filter helper expected a string. Got a number instead.'
    })).calledOnce);
  });

I guess this can be closed now?

@ef4
Copy link
Contributor Author

ef4 commented May 8, 2018

I still think this is a bug. render should throw.

@ef4
Copy link
Contributor Author

ef4 commented May 8, 2018

But thanks for sharing the workaround!

@rwjblue
Copy link
Member

rwjblue commented May 8, 2018

I still think this is a bug. render should throw.

Agreed. Though the actual details of how to implement this properly are non-trivial. I will try to gather better info and report here...

@Frozenfire92
Copy link

👍 Chiming in as someone who is being affected by this

Ember 3.3

Noting neither this workaround or the one @kellyselden mentioned above worked for me.

Noting I'm coming to this not when rendering fails, but when a click throws an error, but assuming its the same issue with the render queue

Test

test('Handles not having action passed in', async function(assert) {
    await render(hbs`{{async-button}}`);

    assert.throws(
      async () => {
        await click(find('.async-button'));
      },
      'Throws exception when action not passed in'
    );
  });

Relevant piece of component (within click handler)

else {
  throw new Error('missing passed in action to async-button');
}

How it looks in qunit browser window
screenshot from 2018-08-02 15-49-51

@nlfurniss
Copy link
Contributor

nlfurniss commented Aug 12, 2018

@Frozenfire92 out of curiosity, what happens if you do this instead:

  assert.throws(
    () => this.$('.async-button').click(),
    'Throws exception when action not passed in'
  );

I'm running into the same issue as you and only using jQuery works; neither click(myElem) nor find(myElem).click() works.

@nlfurniss
Copy link
Contributor

nlfurniss commented Aug 12, 2018

This twiddle reproduces the issue.

TLDR

There is a component that will throw an error if two attrs arent passed in (x and y) and the component is clicked on.

There are three tests:

  1. Ensure no errors if both attrs passed in and component is clicked ✅
  2. Ensure an error is throw is only one attr is passed in and component is clicked (uses native JS and tests fails) ❌
  3. Ensure an error is throw is only one attr is passed in and component is clicked (uses jquery and test passes) ✅

Here is where things go wrong:

// Qunit throws
try {
   block.call(currentTest.testEnvironment);  <====== This call doesnt trigger an error when it uses native JS. But FUN FACT: if you highlight this line at a breakpoint, it will log an error (???)
} catch (e) {
   actual = e;
}
...
if (actual) {     <==== `actual` needs to be set otherwise it wont catch the error and the test will fail.

@Frozenfire92
Copy link

@nlfurniss switching to jquery worked for me, not ideal but a temp fix until this issue is resolved, thanks!

@barryofguilder
Copy link

Are there any updates to this? I was going through a bunch of my tests where they referenced checking up on this issue. Just wanted to make sure there are still no other options than what's mentioned here.

@MrChocolatine
Copy link

Any news about this? After more than 2 years?

@buschtoens
Copy link
Contributor

buschtoens commented Feb 24, 2022

I've started a PR to finally fix this: #1194. I would appreciate your thoughts!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
9 participants