Add failing test around moduleName mutation #14194
Conversation
| @@ -48,6 +48,20 @@ moduleFor('Components test: local lookup', class extends RenderingTest { | |||
| this.assertText('Nested template says: Hi!', 'Re-render works'); | |||
| } | |||
|
|
|||
| ['@htmlbars moduleName remains unchanged']() { | |||
There was a problem hiding this comment.
There ain't no htmlbars up in here
There was a problem hiding this comment.
You will need to target the PR against beta branch to get it to run tests.
There was a problem hiding this comment.
Hmm. I'm happy to land this test here though too. Can you remove the @htmlbars prefix, and open a separate PR against beta?
asakusuma
force-pushed
the
module-name-failing
branch
from
3fb923c to
a17e7d0
Compare
|
|
||
| this.render('{{x-root}}'); | ||
|
|
||
| let moduleName = this.owner.lookup('template:-top-level').meta.moduleName; |
There was a problem hiding this comment.
Looks like this.owner.lookup('template:-top-level') is returning undefined (causing the Travis failures).
asakusuma
force-pushed
the
module-name-failing
branch
from
a17e7d0 to
de6a2ae
Compare
|
Ah, so this test is testing an implementation detail that is engine specific. Meta is stored differently between glimmer and htmlbars. I've updated the test, but I'm thinking it should be glimmer only. |
|
Yeah, we completely removed the htmlbars stuff. So targeting glimmer only is perfect. |
|
|
||
| this.render('{{x-root}}'); | ||
|
|
||
| let moduleName = this.owner.lookup('template:-top-level').spec.meta.moduleName; |
There was a problem hiding this comment.
Maybe we can make moduleName a getter on the template wrapper class...
|
@asakusuma - Can you update this test to work with the recent refactor (making meta available both on the factory and the instance)? We can test that |
|
@asakusuma - Ping? |
|
@rwjblue sorry I totally forgot about this, yes I will do that. |
|
☔ The latest upstream changes (presumably #14432) made this pull request unmergeable. Please resolve the merge conflicts. |
|
@asakusuma - I'm going to go ahead and close this for now, please submit a new PR if you have a chance to pick this up again... |
Failing test that addresses #14192
cc @krisselden @rwjblue @trentmwillis