Store._find asserts adapterPayload not empty

[seanpdoyle]

Currently, Store._find only asserts that the adapterPayload is
non-null (i.e. {} is a valid payload).

This commit forces the adapterPayload to be non-empty.

When TDDing with Mirage, adapterPayload is often {}, which sneaks
through the asserts and ends up as a non-descriptive:

Assertion Failed: You must include an 'id' for undefined in an object
passed to 'push'

Which is triggered in Store._pushInternalModel,
which is a few method invocations away from the source of the issue.

[seanpdoyle]

that could use a better assertion. What adapter are you using?
--@bmac

What would you suggest?

[bmac]

These assertions look great @seanpdoyle. Do you mind adding tests to make sure the assertions are called when the payload is {}? Let me know if you need help doing figuring out where the test goes.

[seanpdoyle]

@bmac I think I'll need some guidance.

I'm surprised that these assertions caused test failures to begin with.

[seanpdoyle]

@bmac how can I exercise this assertion?

[bmac]

@seanpdoyle I would follow the pattern used in this test of stubbing the adapter method to return a value that triggers the assertion then using the expectAssertion helper to test that it is called.

data/tests/integration/adapter/queries-test.js

Lines 47 to 63 in 1d5dfa1

	test("The store asserts when query is made and the adapter responses with a single record.", function(assert) {
	env = setupStore({ person: Person, adapter: DS.RESTAdapter });
	store = env.store;
	adapter = env.adapter;
	adapter.query = function(store, type, query, recordArray) {
	assert.equal(type, Person, "the query method is called with the correct type");
	return Ember.RSVP.resolve({ people: { id: 1, name: "Peter Wagenet" } });
	};
	assert.expectAssertion(function() {
	Ember.run(function() {
	store.query('person', { page: 1 });
	});
	}, /The response to store.query is expected to be an array but it was a single record/);
	});

[seanpdoyle]

@bmac unfortunately, there isn't a store operation that maps directly to this one like the others.

For instance, adapter.findRecord maps to store.find, adapter.query maps to store.query (and maybe store.queryRecord?).

In this case, isn't findMany used to coalesce find requests?

Would I be able to exercise adapter.findMany with a store.query({ ids: [1,2,3] })?

[seanpdoyle]

@bmac I've tried the following:

adapter.findMany = () => Ember.RSVP.resolve({});

assert.expectAssertion(() => {
  run(() => store.query('person', { ids: [1, 2] }));
}, /the adapter's response did not have any data/);

But it didn't raise an error.

[seanpdoyle]

@bmac which store operation maps to a findMany?

[bmac]

I believe you can trigger it by setting coalesceFindRequests: true on the adapter then making 2 findRecord requests in the same run loop.

assert.expectAssertion(() => {
  run(() => {
    store.findRecord('person', 1);
    store.findRecord('person', 2);
  });
}, /the adapter's response did not have any data/);

[seanpdoyle]

@bmac 👍 that did the trick.

[seanpdoyle]

@bmac what's stopping this from being merged?

Is there anything else I can do?

[bmac]

Sorry, @seanpdoyle I've been very busy recently and hadn't had a chance to review this pr. Looking at it now.

[bmac]

Thanks @seanpdoyle. Sorry for the delay.