Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

feat(ngMock): add flushByIndex function #6412

Closed
wants to merge 1 commit into from

Conversation

atogata
Copy link

@atogata atogata commented Feb 23, 2014

Request Type: feature

How to reproduce:

Component(s): ngMock

Impact: small

Complexity: small

This issue is related to:

Detailed Description:

I've recently needed to flush httpBackend requests in an arbitrary order to add unit tests around an edge case. This PR satisfies that need.

Other Comments:

Adds a flushByIndex function to allow pending requests on httpBackend to be flushed in an arbitrary
order.

@mary-poppins
Copy link

Thanks for the PR! Please check the items below to help us merge this faster. See the contributing docs for more information.

  • Uses the issue template (#6412)

If you need to make changes to your pull request, you can update the commit with git commit --amend.
Then, update the pull request with git push -f.

Thanks again for your help!

Adds a flushByIndex function to allow pending requests on httpBackend to be flushed in an arbitrary
order.
@atogata
Copy link
Author

atogata commented Feb 23, 2014

Updated with the issue template

@pkozlowski-opensource
Copy link
Member

@atogata currently the TravisCI build is failing (https://travis-ci.org/angular/angular.js/jobs/19426249) so this PR needs more work.

But besides the build issues I would love to know more about your actual use-case. If I'm not mistaken request are flushed in the order of expectations set so you can control the order by registering expectations. Could you provide an example unit test that is easier to write with the index-based flushing?

@atogata
Copy link
Author

atogata commented Feb 24, 2014

@pkozlowski-opensource Thank you for your response, the exact use case depends highly on some specifics of the system I’m working with, but I will try to explain as clearly as possible.

We have a system that can be called in a manner like:
service.get(‘/events’) called at T=0s
service.get(‘/events/eventKeyA’) called at T=2s

The /events endpoint will always return the result of /events/eventKeyA, along with other entities. Since both endpoints can be slow, we have some logic in place such that service.get(‘/events/eventKeyA’) can return data as soon as it’s available from the /events call and not have to wait for the singular call.

The problem in testing this seems to be that

httpBackend.expectGET(‘/v1.0/events’);
httpBackend.expectGET(‘/v1.0/events/eventKeyA’);
service.get(‘/events’);
service.get(‘/events/eventKeyA’);

will cause /events to be flushed first, even though this particular test needs /events/eventKeyA to be flushed first.

httpBackend.expectGET(‘/v1.0/events/eventKeyA’);
httpBackend.expectGET(‘/v1.0/events’);
service.get(‘/events’);
service.get(‘/events/eventKeyA’);

The alternative above seems to cause the test to throw an error,
Error: Unexpected request: GET /v1.0/events
Expected GET /v1.0/events/eventKeyA

As for the failing Travis CI build, I did push up a new commit with the corresponding build https://travis-ci.org/angular/angular.js/builds/19426708, however it seems like 9039.2 hung. Would you be able to restart it?

@atogata
Copy link
Author

atogata commented Feb 24, 2014

Also, included below is the actual test using this function.

    it('should use the result of the singular response, if it resolves before the parent response', function() {
        var COURSES_RESPONSE = _makeHttpCollectionSuccessResponse([COURSE_A_RESOURCE, COURSE_B_RESOURCE, COURSE_C_RESOURCE]);

        httpBackend.expectGET('/v1.0/courses').respond(angular.copy(COURSES_RESPONSE));
        httpBackend.expectGET('/v1.0/courses/course_resource_key_a').respond(angular.copy(_extendResponse(COURSE_A_RESPONSE, {
            'singularResponse': true,
        })));

        instance.get('/v1.0/courses').then(resolveCallback, rejectCallback);
        instance.get('/v1.0/courses/course_resource_key_a').then(resolveCallback2, rejectCallback2);

        httpBackend.flushByIndex(1);

        expect(resolveCallback).not.toHaveBeenCalled();

        var result2 = resolveCallback2.mostRecentCall.args[0];

        expect(result2.singularResponse).toBeTruthy();

        httpBackend.flush();
    });

@mary-poppins
Copy link

I'm sorry, but I wasn't able to verify your Contributor License Agreement (CLA) signature. CLA signature is required for any code contributions to AngularJS.

Please sign our CLA and ensure that the CLA signature email address and the email address in this PR's commits match.

If you signed the CLA as a corporation, please let us know the company's name.

Thanks a bunch!

PS: If you signed the CLA in the past then most likely the email addresses don't match. Please sign the CLA again or update the email address in the commit of this PR.
PS2: If you are a Googler, please sign the CLA as well to simplify the CLA verification process.

@atogata atogata added cla: no and removed cla: yes labels Feb 26, 2014
@btford btford added this to the Backlog milestone Mar 17, 2014
@btford btford removed their assignment Mar 17, 2014
@atogata atogata added cla: no and removed cla: yes labels Mar 18, 2014
@atogata atogata closed this Mar 19, 2014
@atogata atogata reopened this Mar 19, 2014
@atogata
Copy link
Author

atogata commented Mar 19, 2014

@mary-poppins I've signed the CLA again under the correct e-mail.

@pkozlowski-opensource CI build passed (https://travis-ci.org/angular/angular.js/builds/21078576), are there any updates on the status of this PR? Thank you for your time!

@mary-poppins
Copy link

CLA signature verified! Thank you!

Someone from the team will now triage your PR and it will be processed based on the determined priority (doc updates and fixes with tests are prioritized over other changes).

@Narretz
Copy link
Contributor

Narretz commented Aug 1, 2016

Superseded by #14967

@Narretz Narretz closed this Aug 1, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants