return removed fixtures when adding new fixtures #154
Conversation
|
WOW! I just noticed this. Thank you so much! |
core.js
Outdated
| } | ||
|
|
||
| // Adds a fixture to the list of fixtures. | ||
| exports.add = function (settings, fixture) { | ||
|
|
||
| if (Array.isArray(settings)) { |
There was a problem hiding this comment.
When does this case get called?
test/fixture_test.js
Outdated
| xhr.open('GET', 'services/thing'); | ||
| xhr.send({}); | ||
|
|
||
| // fixture("services/thing", function(){ ... CODE B ... }); |
There was a problem hiding this comment.
cruft needing to be cleaned up?
|
@randallwhitlock, @cherifGsoul is going to work on this Monday. Those comments were for him. Thanks again for submitting! |
cherifGsoul
force-pushed
the
34-oldfixtures
branch
from
f42ea49 to
13dcda3
Compare
|
@justinbmeyer I cleaned up the code, can you please review? |
The context
Expected behavior:
The changes I made after @randallwhitlock commit :
|
|
@randallwhitlock Thank you for this, we still working on it. |
test/fixture_test.js
Outdated
| start(); | ||
| }); | ||
| xhr.open('GET', '/services/thing'); | ||
| xhr.send(); |
There was a problem hiding this comment.
Is the XHR needed? If so, I don’t understand what’s going on here and we should add some more comments to make it clear.
There was a problem hiding this comment.
@chasenlehara can you please explain what is not clear? I just hit the fixture under the test with XHR request.
There was a problem hiding this comment.
Here’s my understanding of what’s happening in this test:
- Setting up a fixture
- Removing the fixture
- Defining a new fixture
- Testing that the old fixture was returned when it was removed
- Setting up the old fixture again
- Making an XHR that will call the old fixture
I’m not sure why steps 3, 5, and 6 happen, because only 1, 2, and 4 seem necessary for the test.
There was a problem hiding this comment.
@chasenlehara Thank you, I changed it.
It was just to make sure the returned fixture really works as expected.
#34 Changed fixture to return removed fixture settings.