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

Incompatibile with RFC 232 #18

Open
alexdiliberto opened this issue Oct 24, 2017 · 9 comments
Open

Incompatibile with RFC 232 #18

alexdiliberto opened this issue Oct 24, 2017 · 9 comments

Comments

@alexdiliberto
Copy link

alexdiliberto commented Oct 24, 2017

The RFC essentially makes rendering async, which means that if you were doing:

  assert.expectAssertion(() => {
    this.render(hbs`stuff here`);
  });

Then the codemod "helpfully" transforms that into:

  assert.expectAssertion(async () => {
    await render(hbs`stuff here`);
  });

But this library always expects the callback to trigger assertions synchronously.


Further reading:

@rwjblue
Copy link
Collaborator

rwjblue commented Oct 24, 2017

Updating issue description...

@rwjblue
Copy link
Collaborator

rwjblue commented Oct 25, 2017

Was chatting with @workmanw in slack about this, and came up with master...async-wip as a possible solution for expectAssertion.

@rwjblue
Copy link
Collaborator

rwjblue commented Oct 25, 2017

I think next steps are to put some actual tests together and get the other assertion helper methods in line with that WIP (and fix my typos in the WIP).

Any takers?

@alexdiliberto
Copy link
Author

alexdiliberto commented Oct 25, 2017

I was taking a bit of a stab at it, issue I'm running into is that render now returns a promise which has it's body execute on the next iteration of the runloop. (https://github.com/emberjs/ember-test-helpers/blob/v0.7.0-beta.8/addon-test-support/setup-rendering-context.js#L98) So the expectAssertion code will always return before the component's init() hook has even fired.

I'll push up what I have even though it's not complete...#21

@heathharrelson
Copy link

Our apps make fairly extensive use of assertions to document assumptions. I tried upgrading one of my apps to the new testing API while I was at EmberConf, and this broke around 100 of the tests. Do you have any idea how to get this working with the new API? Is there anything I could do to help?

@kturney
Copy link

kturney commented May 17, 2018

@rwjblue provided a solution here https://youtu.be/PwQAj5UU9ng?t=1h25m30s.

Taking that info, I've made the following helper for my tests:

import { waitUntil } from 'ember-native-dom-helpers';
import Ember from 'ember';

export default function waitForError(opts) {
  const orig = Ember.onerror;

  let error = null;
  Ember.onerror = (err) => {
    error = err;
  };

  return waitUntil(() => error, opts).finally(() => {
    Ember.onerror = orig;
  });
}

I then replace

assert.expectAssertion(() => {
  render(hbs`
    {{button-hero
      align='center'
      icon='svg-add'
      theme='secondary'
      text='Create New Group'
    }}
  `);
}, 'Assertion Failed: An action is required');

with

const [ err ] = await Promise.all([
  waitForError(),
  render(hbs`
    {{button-hero
      align='center'
      icon='svg-add'
      theme='secondary'
      text='Create New Group'
    }}
  `)
]);

assert.equal(err.message, 'Assertion Failed: An action is required', 'correct assertion');

maybe there's a way to wrap up that same logic into something that looks more like assert.expectAssertion

@heathharrelson
Copy link

heathharrelson commented Nov 7, 2018

EDIT: Note that my solution below didn't work anymore after updating to use @ember/test-helpers.

For what it's worth, I ended up working around this in my projects by just wrapping the render call in a try and asserting the content of the message in the catch block:

test('it throws an exception if no range validation is present', async function(assert) {
  try {
    await render(hbs`{{clipped-numeric options=options}}`);
  } catch (e) {
    assert.equal(e.message, 'Assertion Failed: A single optional_in_range validation is required');
  }
});

@shokmaster
Copy link

Any news about this issue?

@RobbieTheWagner
Copy link

Would love to see support for this baked in. Anything we can do to get this over the finish line?

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

6 participants