Fixing tests for Ember Data 1.0.0-beta.5

[kurko]

This PR has a bunch of changes required to work with the new architecture used since ED entered beta.

• Fix find() tests
• Fix findMany() tests (this one is tough)
• Fix findQuery() tests
• Fix findAll() tests
• Fix createRecord() tests
• Fix updateRecord() tests
• Fix deleteRecords() tests
• Fix bulkCommits() tests (not sure this one is needed anymore)
• Fix load has many tests
• Fix load belongs to tests
• Fix save relationship tests
• Refactor crazy helpers
• Remove memoized variables that prevent data from being truly stored (closes Relations lost on page reload #23)

[kurko]

@fivetanley @rpflorence ping

[kurko]

I'm still struggling with this test. Can't make it pass nor understand why. Basically, ED throws all sorts of errors.

I'm thinking about removing this test, given no other adapter tests it directly. Then I'll probably Unit test the method in another test namespace.

[amiel]

I agree with these statements as being pretty good reasons to just delete this test and check off "Fix findMany() tests (this one is tough)".

[amiel]

For "Refactor crazy helpers", is there anything in particular you are looking for? I just deleted a bunch of unused stuff. I figure it's ok-ish now (see carnesmedia/ember-localstorage-adapter@22cf6d162fa87f70175be9e397ae4fb4e5b00011)

[kurko]

Yeah, I'd remove the helpers that make reading tests harder. I personally don't like assertStoredList, for example, because you don't know what's really being tested unless you go check that helper. I agree reusability is important, but I think tests should be as explicit as possible.

Regardless of that, I think these changes in the helpers are definitely not as important as making the tests pass. I'll trust your judgement on this one.

[amiel]

FYI: there are a bunch of changes in kurko#1

[kurko]

Thanks for the hard work, @amiel! Are the tests passing? (I'm going to download it tonight and run them)

[kurko]

Ok, so I unchecked load hasMany because it's broken (at least for me). Also, given we're on a shared branch, remember to not change history of commits.

[kurko]

Guys, good news! All tests are finally passing locally. I want to thank @amiel and @bjarkehs immensely for the hard work. After all this coordinated effort, seeing the final result is what makes me love open source.

Now, before merging, I'd like to ask you guys to review this code. Also, we need to change the README with things like "{async: true}" is not needed and some other things. It's late here, so I won't do it now. I'm also going to push a fix for the phantomjs runner.js file, that's broken since forever.

Thank you.

[kurko]

Although fixing TravisCI is out of the scope of this PR, merging the runner.js file into master wouldn't serve us anything, given all tests are failing.

[amiel]

YAY! Thanks @bjarkehs and @kurko!

[amiel]

Is there anything left to do for this pull-request?

[kurko]

I'll review it tonight and then update the README.

[kurko]

We should probably add a comment here on why we're extracting it the way we're doing it.

[fivetanley]

To everyone helping out, you have my deepest gratitude. I'll give this a formal review as soon as I can, hopefully tomorrow morning before work. I am so happy tears are running down my face.

[bjarkehs]

I'm not sure if this is a bug with the adapter or my code, but I'm getting the error:

Assertion failed: You looked up the 'hours' relationship on '<App.Order:ember356:jbti6>' but some of the associated records were not loaded. Either make sure they are all loaded together with the parent record, or specify that the relationship is async (`DS.hasMany({ async: true })`)

In order for this to make sense I have the following models defined:

App.Order = DS.Model.extend({
    navision: DS.attr('number'),
    user: DS.belongsTo('user'),
    hours: DS.hasMany('hour'),
    materials: DS.hasMany('material'),
    services: DS.hasMany('service')
});
App.Hour = DS.Model.extend({
    web_id: DS.attr('number'),
    amount: DS.attr('number'),
    overtime1: DS.attr('number'),
    overtime2: DS.attr('number'),
    date: DS.attr('string'),
    order: DS.belongsTo('order')
});

I'd think that since everything is local we might as well fetch it all? When I console.log the order object I can access the hours through _data and then hours, so I'm not entirely sure where it goes wrong. Anyone with some insight on this?

[kurko]

Merged this. Finding new issues and opening PRs will be easier now. I copied master into a branch called pre-beta.

@bjarkehs is that error occurring in the tests? If so, have you registered the App.Hour model in the store? Maybe could you open a new issue?