[CHORE tests] modernize integration/adapter/find-all-test #5737
Conversation
|
|
||
| module('integration/adapter/find-all - Finding All Records of a Type', function(hooks) { | ||
| setupTest(hooks); | ||
| let store, allRecords; |
There was a problem hiding this comment.
we should be able to (or should make ourselves able to) eliminate allRecords being scoped so broadly.
| }); | ||
| store = env.store; | ||
| }, | ||
| hooks.afterEach(function() { |
There was a problem hiding this comment.
this afterEach is no longer needed at all
| adapter.findAll = () => resolve({}); | ||
|
|
||
| assert.expectAssertion(() => { | ||
| run(() => store.findAll('person')); |
There was a problem hiding this comment.
we can clean this run up in another pass but I believe either expectAssertion itself or a similar util is promise aware that we can use instead.
There was a problem hiding this comment.
I'm not sure what's the best approach here, more info at workmanw/ember-qunit-assert-helpers#18.
There was a problem hiding this comment.
@thorsteinsson the try .. catch approach is amenable.
| return store.findAll('person').then(persons => { | ||
| assert.equal(persons.get('isUpdating'), false); | ||
| assert.equal(persons.get('length'), 2); | ||
| }); |
There was a problem hiding this comment.
A better pattern than:
let wait = run(() => return new Promise())
// ... assert intermediate state
await wait;
is
let promise = new Promise()
// ... assert intermediate state
await promise;
mostly this just means you can remove the run here. 👍
|
|
||
| return run(() => { | ||
| return store.findAll('person').then(persons => { | ||
| return run(() => { |
There was a problem hiding this comment.
we should be able to eliminate this use of run
|
Thanks for the comments, I'll address them. |
|
Thanks a bunch for the PR! Can you please rebase it against master, there are some merge conflicts with master right now. |
runspired
force-pushed
the
find-all-tests
branch
from
c9c2d94 to
5012e4d
Compare
No description provided.