Update ember-cli and fix failing tests#16465
Update ember-cli and fix failing tests#16465rwjblue merged 1 commit intoemberjs:masterfrom GavinJoyce:gj/update-ember-cli
Conversation
GavinJoyce
changed the title
Yes. |
|
|
||
| describe('with usePods=true', function() { | ||
| //TODO: GJ: figure out what changed here | ||
| describe.skip('with usePods=true', function() { |
There was a problem hiding this comment.
Maybe one of these:
There was a problem hiding this comment.
Thanks, a git bisect points to ember-cli/ember-cli@5d5801a. I'm not yet clear why though
There was a problem hiding this comment.
Yes, removing the caching "fixes" the issue
There was a problem hiding this comment.
oh, there are some details on this in the previous PR: ember-cli/ember-cli#7491 (comment)
There was a problem hiding this comment.
I think in general that the caching is fine, we just need an escape hatch to bypass and reset...
tldr; cache invalidation is hard
There was a problem hiding this comment.
@patocallaghan @rwjblue perhaps we could revert the feature for now? It looks like we haven't announced it yet. This will unblock MU blueprints and allow us time to come back and add a more robust implementation of the feature
There was a problem hiding this comment.
@GavinJoyce I wasn't aware that the tests had been moved elsewhere, I just thought they'd been deleted.
I had a look there and I remember it being quite tricky to make those particular tests work with get-config. The config had already been cached at the module level and the replaceFile logic in the test happened too late. It wasn't easy to "reach in". In other tests that tested this sort of thing we used multiple fixture directories so it worked fine.
We're not actually using this feature ourselves yet so if it unblocks your MU work I'm fine for the changes to be reverted for now.
@rwjblue any thoughts? another option is to remove the cache but I believe the instrumentation lib will call getConfig a lot which will result in lots of Yam objects being created.
There was a problem hiding this comment.
I don't think we need to cache there, it was just a "nice to have". Though its possible I am missing a caveat of the other changes that may have required caching...
There was a problem hiding this comment.
GavinJoyce
changed the title
|
@rwjblue this is now 🍏 and ready for review |
|
|
||
| import { initialize } from '<%= dasherizedModulePrefix %>/initializers/<%= dasherizedModuleName %>'; | ||
| import { module, test } from 'qunit'; | ||
| <% if (destroyAppExists) { %>import destroyApp from '../../helpers/destroy-app';<% } else { %>import { run } from '@ember/runloop'; <% } %> |
There was a problem hiding this comment.
run is already imported above
GavinJoyce
changed the title
|
Thank you @GavinJoyce! |
3 participants
In order to continue working on MU Blueprints, we need to update the ember-cli version that we're using in ember.js.
The output of
ember newhas since been changed to emit new style tests. These changes result in a number of failing ember.js tests. For example, thedestroyApphelper no longer exists on disk so the default generated test format output has changes for a bunch ofember generateblueprints.I've updated the existing tests to expect the new default output and I've pinned
ember-cliat this SHA.I've also removed ember-cli config caching which was also causing failing tests ember-source