Decouple test from the default resolver for data_adapter#15091
Decouple test from the default resolver for data_adapter#15091mixonic merged 1 commit intoemberjs:masterfrom
Conversation
mixonic
left a comment
mixonic
left a comment
There was a problem hiding this comment.
I believe that first test can just have its description changed. A read of https://github.com/emberjs/ember.js/blob/master/packages/ember-extension-support/lib/data_adapter.js doesn't make me believe these tests are specific to the default resolver, nor do these tests.
Some small cleanup remains. Thanks @Gaurav0!
| App.destroy(); | ||
| init() { | ||
| this._super(...arguments); | ||
| let self = this; |
There was a problem hiding this comment.
I don't believe self is used here
|
|
||
| QUnit.test('getRecords gets a model name as second argument', function() { | ||
| App.Post = Model.extend(); | ||
| ['@test Model types added with DefaultResolver']() { |
There was a problem hiding this comment.
So this test is specifically mentioning DefaultResolver. By migrating to the ApplicationTestCase and using add etc, it is no longer using the default resolver. Instead it now uses the TestResolver.
Either the test description is wrong and the data adapter doesn't require the DefaultResolver, or this test should not be ported. I''m not sure which.
There was a problem hiding this comment.
Removing DefaultResolver from description
| init() { | ||
| this._super(...arguments); | ||
| let self = this; | ||
| this.set('containerDebugAdapter', { |
There was a problem hiding this comment.
I don't think this property is used?
There was a problem hiding this comment.
Tests fail without a mock containerDebugAdapter.
There was a problem hiding this comment.
I misread this the first time, looks good 👍
| import EmberDataAdapter from '../data_adapter'; | ||
| import { Application as EmberApplication, Resolver as DefaultResolver } from 'ember-application'; | ||
| import { moduleFor, ApplicationTestCase } from 'internal-test-helpers'; | ||
| import { ModuleBasedResolver } from 'internal-test-helpers/test-resolver'; |
Apply Matthew's comments
|
Thank you @Gaurav0! |
2 participants
For #15058