Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

unloadAll destroys records and an assertion fails in flushCanonical #2982

Closed
givanse opened this issue Apr 8, 2015 · 12 comments
Closed

unloadAll destroys records and an assertion fails in flushCanonical #2982

givanse opened this issue Apr 8, 2015 · 12 comments

Comments

@givanse
Copy link

givanse commented Apr 8, 2015

I have a couple of acceptance tests that pass if run by themselves, but fail when all the tests are executed together. The error:

beforeEach failed on visiting / second time: Assertion Failed: Cannot call get with 'isDeleted' on an undefined object.

The failure happens at a portion of code that was added by this bug fix: bmac@c54323b

What is happening in my tests is that when the application is destroyed unloadAll() is being called and each record is being deleted like this:

record.unloadRecord()
record.destroy()

So, when we reach flushCanonical in this line:

return !record.get('isDeleted');

The assertion fails because the record doesn't have the isDelete property, but it has set to true isDestroyed and isDestroying.

Currently I have a workaround/hack that allows all the tests to pass:

afterEach: function() {
  var store = application.registry.lookup('store:main');
  Ember.run(function() {
    var records = store.all('someModel');
    records.forEach(function(record) {
      store.unloadRecord(record);
    });
  });
  Ember.run(application, 'destroy');
}
@givanse
Copy link
Author

givanse commented Apr 8, 2015

I tried to build a test case (I wanted to submit a PR), but I haven't been able to reproduce the problem without creating and destroying an entire application...

I prepared a branch from my project where the issue reproduces with a minimal set of data.

git clone -b test-case [email protected]:givanse/mvc-tree.git

In file mvc-tree/tests/acceptance/index-test.js you can see the assertion failure if you remove the workaround.

In the model I'm using a hasMany relationship that is async and polymorphic. If the records (not the definition in the model) lose that piece of data, the tests pass even without the workaround I currently have.

@igorT
Copy link
Member

igorT commented Apr 12, 2015

Hi, thanks for reporting the issue. I am having trouble running your test case, when I do ember serve in the repo after npm&bower install I get

➜  mvc-tree git:(test-case) ember serve

Missing npm packages:
Package: ember-data
  * Specified: components/ember#canary
  * Installed: (not installed)

Run `npm install` to install missing dependencies.

@givanse
Copy link
Author

givanse commented Apr 12, 2015

Sorry about that, I just fixed the branch.

git pull origin test-case

Probably not relevant, but I'll mention that the data is being preloaded from an initializer: https://github.com/givanse/mvc-tree/blob/test-case/app/initializers/preload-data.js

@givanse
Copy link
Author

givanse commented Apr 12, 2015

The test http://localhost:4200/tests?nocontainer&hidepassed&module=Acceptance%3A%20Index
file mvc-tree/tests/acceptance/index-test.js

@piotrpalek
Copy link

I encountered an error when unloading records as well and managed to distrill it into a jsbin: http://emberjs.jsbin.com/mazopeburo/2/edit

Maybe this will help you as it's something similiar.

@pangratz
Copy link
Member

@piotrpalek awesome JSBin to demonstrate the issue. I updated ember-data to use the references branch and then the bug is gone ... http://emberjs.jsbin.com/xutopohevu/1/edit?js,console,output

@piotrpalek
Copy link

Np :) @pangratz you sure about that? I click "unload post" then "refresh" and still get Error: Assertion Failed: calling set on destroyed object... in your jsbin.

@pangratz
Copy link
Member

Oh boy, you're right. I guess I didn't look too closely 😢 Sorry for the false alarm.

@igorT
Copy link
Member

igorT commented Jun 3, 2015

I think this might be solved on master, can you check please?

@givanse
Copy link
Author

givanse commented Jun 4, 2015

I just took master for a spin and it seems like the flushCanonical problem is solved, but something else popped up. I'm still digging, but I'll share the new error in case I'm missing something obvious.

Error: Assertion Failed: Passing classes to store methods has been removed. Please pass a dasherized string instead of mvc-tree@model:column:

@givanse
Copy link
Author

givanse commented Jun 4, 2015

Waiting for #3192

@igorT
Copy link
Member

igorT commented Jun 11, 2015

I believe this was fixed as #3192 is closed. Please reopen if it still an issue

@igorT igorT closed this as completed Jun 11, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants