Helper test #15937
Helper test #15937
Conversation
snewcomer
force-pushed
the
helper-test
branch
from
3448fec to
a1c4629
Compare
| @@ -0,0 +1,12 @@ | |||
| import { fooBarBaz } from 'my-app/helpers/foo/bar-baz'; | |||
There was a problem hiding this comment.
@rwjblue should we look this up on the container instead?
There was a problem hiding this comment.
https://github.com/rwjblue/ember-qunit-codemod/blob/master/__testfixtures__/ember-qunit-codemod/native-qunit-to-nested.output.js would suggest no
but I guess in the above scenarios we aren't really testing the helper itself, just the underlying function.
I have found it kind of hard to test class based helpers -- for example:
this.render(hbs`{{my-helper data}}`);may return objects that don't serialize well.
Would be great if we could do something like
this.render(hbs`{{let actualResult (my-helper data)}}` );
assert.equal(this.actualResult, expectedResult);There was a problem hiding this comment.
If you're returning something other than a string or number you should probably not use a rendering test and use a "unit" test instead. In any case both tests should probably use the container. Rendering tests do that implicitly and unit tests explicitly.
There was a problem hiding this comment.
@Turbo87 should I change the unit test to use the container then using factoryFor then?
There was a problem hiding this comment.
Not right now, lets focus on 1:1 for now then we can follow up with a PR that adds usage of owner.factoryFor('helper:foo').create().
There was a problem hiding this comment.
I've discussed this at length with @krisselden and I believe he is opposed to the public blueprints instantiating helpers outside of the confines of the template layer. I personally disagree, but I don't want to make a change and "sneak it in" without a more thorough discussion. Does that make sense?
|
|
||
| await render(hbs`{{foo/bar-baz inputValue}}`); | ||
|
|
||
| assert.equal(this.$().text().trim(), '1234'); |
There was a problem hiding this comment.
please use this.element.textContent.trim() instead to avoid jQuery
snewcomer
force-pushed
the
helper-test
branch
2 times, most recently
from
6399b0b to
cd6b8d4
Compare
| return emberGenerateDestroy(['helper-test', 'foo/bar-baz', '--unit'], _file => { | ||
| expect(_file('tests/unit/helpers/foo/bar-baz-test.js')) | ||
| .to.equal(fixture('helper-test/rfc232-unit.js')); | ||
| }); |
There was a problem hiding this comment.
I've found the default as integration to be confusing for helpers. But that's probably just me though 🤷♂️
There was a problem hiding this comment.
Helpers are only logically used from a template, testing them outside of the template layer should be a fairly uncommon thing to do...
| @@ -0,0 +1,12 @@ | |||
| import { fooBarBaz } from 'my-app/helpers/foo/bar-baz'; | |||
There was a problem hiding this comment.
@Turbo87 should I change the unit test to use the container then using factoryFor then?
|
Thank you @snewcomer! |
Ref #15933
- [x] fixture helper tests (including mocha unit tests)recommended to do in a separate PR