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

Added failing test for issue #5025. #5095

Closed
wants to merge 1 commit into from

Conversation

workmanw
Copy link

@workmanw workmanw commented Jul 25, 2017

Ping @stefanpenner -- As promised, a failing test.

One thing to note is this assertion:

    assert.equal(get(people, 'length'), 2, 'Unloaded record removed from the array');

This would technically have failed before the previous regression because instead of nullifying the record in the list, it just left the stale value until the end of the loop. If the goal is to restore that behavior, then we need to change the test criteria.

And I would be happy to do that, let me know.


For reference a test of the previous functionality would look like:

run(() => {
  people.objectAt(0).unloadRecord();
  
  assert.equal(get(people, 'length'), 3, 'Unload does not complete until the end of the loop');
  assert.ok(get(people.objectAt(0), 'name'), 'Scumbag Dale', 'Dale is still the first object until the end of the loop');
});

assert.equal(get(people, 'length'), 2, 'Unloaded record removed from the array');
assert.ok(get(people.objectAt(0), 'name'), 'Scumbag Katz', 'Katz shifted down after the unload');
assert.ok(get(people.objectAt(1), 'name'), 'Scumbag Bryn', 'Bryn shifted down after the unload');

@runspired
Copy link
Contributor

I ported this test into #5378 and confirmed that the fix for #5350 also fixes this issue.

@runspired runspired closed this Mar 17, 2018
bmac pushed a commit that referenced this pull request Mar 26, 2018
* [BUGFIX] resolve issues with RecordArray sync for peekAll

* remove unneeded part of fix

* Adds a test for #5167

* add test and fix for #5350

* port test from #5095

* fix filter test for older versions of Ember
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

Successfully merging this pull request may close these issues.

None yet

2 participants