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

Wait for mocked XHR to fulfil all requests #71

Open
serbanghita opened this issue Oct 31, 2018 · 7 comments
Open

Wait for mocked XHR to fulfil all requests #71

serbanghita opened this issue Oct 31, 2018 · 7 comments
Labels

Comments

@serbanghita
Copy link
Contributor

serbanghita commented Oct 31, 2018

I'm having the following situation:

function reloadApp() {
   $.ajax({url: "first/url"}).then(() => {

       $.ajax({url: "second/url"}).then(() => updateSomeDOMAttributes());

   }).then((r1) => addSomeDOMAttributes());

   renderDOMComponent();
}

There are two ajax requests nested but not chained to one another.
I'm using Sinon.js (which uses nise).

I have no way of waiting for the second request to fulfil unless I chain the Promise which is something I cannot do without breaking compatibility (let's assume that it cannot be done).

    beforeEach(() => {
        server = sinon.createFakeServer({respondImmediately: true});
        spy = sinon.spy(tracking, "trackEvent");
    });

    afterEach(() => {
        server.restore();
        spy.restore();
    });

    it('ajax mocking works #1', (done) => {
    
         server.respondWith("GET", 'first/url', [ 200, { "Content-Type": "text/html" }, `mocked1` ]);
         server.respondWith("GET", 'second/url', [ 200, { "Content-Type": "text/html" }, `mocked2` ]);
    
         reloadApp().then((r1) => {
             expect(r1).to.equal(`mocked1`);
             expect(spy.callCount).to.equal(2); <------- fails
             done();
         });
    
         // server.respond();
    
     });

I tried both strategies with respondImmediately and with the server.respond().

My solution is to make a helper:

/**
 * Wait for the Sinon.js nise XHR mocks to finish their registered
 * responses.
 *
 * @param server Server created with sinon.createFakeServer
 * @param timeout Optional, how much to wait before failing.
 * @returns {Promise<any>}
 */
function waitForServerToFulfillRequests(server, timeout) {
    timeout = timeout || 100;
    return new Promise((resolve, reject) => {
        let checkCount = 0;
        const i = setInterval(() => {
            if (server.requests.length === server.responses.length) {
                clearInterval(i);
                resolve();
            }
            if (checkCount === 100) {
                clearInterval(i);
                reject(`Maximum waiting count of ${checkCount * 16} has passed.`);
            }
            checkCount++;
        }, 16);
    });
}

This is obviously an edge case but my goal is to test the final outcome of the code and wait for all the requests to finish because they modify the HTML component that I'm testing.

Working example: https://github.com/serbanghita/karma-mocha-chai-sinon ;

Let me know your thoughts on the situation and waitForServerToFulfillRequests

@fatso83
Copy link
Contributor

fatso83 commented Oct 31, 2018

This is like lolex's runAllTimers. Seems like a nice addition?

P.S. For reloadApp().then((r1) ... to work your reloadApp function needs to return something.

@serbanghita
Copy link
Contributor Author

@fatso83 thanks for pointing that out. I can make a PR, i'm trying to understand how to test this in isolation and in the cotext of Sinon afterwards. I want the timeout property to be dictated from the frameworks timeout settings.

On reloadApp, I didn't care about testing the return value of that, the use case is more about "waiting for all the XHRs to finish"

@fatso83
Copy link
Contributor

fatso83 commented Oct 31, 2018

There's too many unclear things you refer to for me to understand what you talk about.

I want the timeout property to be dictated from the frameworks timeout settings.

Which timeout property are you talking about?
The autoRespondAfter timeout? Or the timeout parameter in your example function waitForServerToFulfillRequests?

Which framework are you talking about?
The testing framework? If so, please don't. We don't want test details to be connected with details of Mocha/Ava/... or something else. I don't see any mention of a timeout above. Also, I don't think Mocha exposes hooks to read it, only set it using this.timeout(millis)

On reloadApp, I didn't care about testing the return value of that

What is "that" referring to? If you refer to my pointing out you need to return a value, it had nothing to do with testing and everything to do with the fact that your example wasn't runnable, as the .then() would result in a NullPointer exception.

@serbanghita
Copy link
Contributor Author

serbanghita commented Nov 7, 2018

@fatso83 I'll try to clarify:

On timeout property that waitForServerToFulfillRequests uses.

My original goal was to have a way to stop the setInterval checks.
The scenario in my mind was: the tests have failed (eg. 2000 ms passed) and I still do the checks, I want to stop. Testing frameworks allow tests to run even if one failed so I don't want to leave this check in the background.

I could stop the setInterval checks inside waitForServerToFulfillRequests by using a checkCount equal to autoRespondAfter * this.responses.length + 1.

If you refer to my pointing out you need to return a value, it had nothing to do with testing and everything to do with the fact that your example wasn't runnable, as the .then() would result in a NullPointer exception.

I updated the example:

function reloadApp() {
   $.ajax({url: "first/url"}).then(() => {
       $.ajax({url: "second/url"}).then((htmlFragment) => updateDOM(htmlFragment));

      return "<markup />";
   }).then((htmlFragment) => updateDOM(htmlFragment));
}

updateDOM - this can do simple stuff like .appendChild or create/update custom web components.

PS: you can run the POC here https://github.com/serbanghita/karma-mocha-chai-sinon

@stale
Copy link

stale bot commented Jan 6, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jan 6, 2019
@fatso83
Copy link
Contributor

fatso83 commented Jan 6, 2019

This has a PR that's waiting review.

@stale stale bot removed the stale label Jan 6, 2019
@stale
Copy link

stale bot commented Mar 7, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Mar 7, 2019
@stale stale bot closed this as completed Mar 14, 2019
@fatso83 fatso83 added pinned and removed stale labels Mar 15, 2019
@fatso83 fatso83 reopened this Mar 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants