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

feat: Wait for all handled requests to resolve via .flush() #75

Merged
merged 7 commits into from
Jul 26, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 49 additions & 5 deletions packages/@pollyjs/adapter-puppeteer/src/index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import Adapter from '@pollyjs/adapter';

const LISTENERS = Symbol();
const POLLY_REQUEST = Symbol();
const PASSTHROUGH_PROMISE = Symbol();
const PASSTHROUGH_PROMISES = Symbol();
const PASSTHROUGH_REQ_ID_HEADER = 'x-pollyjs-passthrough-request-id';
Expand Down Expand Up @@ -73,24 +74,67 @@ export default class PuppeteerAdapter extends Adapter {
const request = response.request();

// Resolve the passthrough promise with the response if it exists
request[PASSTHROUGH_PROMISE] &&
if (request[PASSTHROUGH_PROMISE]) {
request[PASSTHROUGH_PROMISE].resolve(response);

delete request[PASSTHROUGH_PROMISE];
delete request[PASSTHROUGH_PROMISE];
}
},
requestfinished: request => {
// Resolve the deferred pollyRequest promise if it exists
if (request[POLLY_REQUEST]) {
request[POLLY_REQUEST].promise.resolve();
delete request[POLLY_REQUEST];
}
},
requestfailed: request => {
// Reject the passthrough promise with the error object if it exists
request[PASSTHROUGH_PROMISE] &&
if (request[PASSTHROUGH_PROMISE]) {
request[PASSTHROUGH_PROMISE].reject(request.failure());
delete request[PASSTHROUGH_PROMISE];
}

delete request[PASSTHROUGH_PROMISE];
// Reject the deferred pollyRequest promise with the error object if it exists
if (request[POLLY_REQUEST]) {
request[POLLY_REQUEST].promise.reject(request.failure());
delete request[POLLY_REQUEST];
}
},
close: () => this[LISTENERS].delete(page)
});

this._callListenersWith('prependListener', page);
}

onRequest(pollyRequest) {
const [request] = pollyRequest.requestArguments;

/*
Create an access point to the `pollyRequest` so it can be accessed from
the emitted page events
*/
request[POLLY_REQUEST] = pollyRequest;
}

/**
* Override the onRequestFinished logic as it doesn't apply to this adapter.
* Instead, that logic is re-implemented via the `requestfinished` page
* event.
*
* @override
*/
onRequestFinished() {}

/**
* Abort the request on failure. The parent `onRequestFailed` has been
* re-implemented via the `requestfailed` page event.
* @override
*/
async onRequestFailed(pollyRequest) {
const [request] = pollyRequest.requestArguments;

await request.abort();
}

async onRecord(pollyRequest) {
await this.passthroughRequest(pollyRequest);
await this.persister.recordRequest(pollyRequest);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,4 +64,40 @@ describe('Integration | Puppeteer Adapter', function() {
setupPolly.afterEach();

adapterTests();

it('should have resolved requests after flushing', async function() {
// Timeout after 500ms since we could have a hanging while loop
this.timeout(500);

const { server } = this.polly;
const requests = [];
const resolved = [];
let i = 1;

server
.get(this.recordUrl())
.intercept(async (req, res) => {
await server.timeout(5);
res.sendStatus(200);
})
.on('request', req => requests.push(req));

this.page.on('requestfinished', () => resolved.push(i++));

this.fetchRecord();
this.fetchRecord();
this.fetchRecord();

// Since it takes time for Puppeteer to execute the request in the browser's
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should document this edge case

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed, there are no currently known edge cases. This is a test nuance that only relates to this test scenario.

// context, we have to wait until the requests have been made.
while (requests.length !== 3) {
await server.timeout(5);
}

await this.polly.flush();

expect(requests).to.have.lengthOf(3);
requests.forEach(request => expect(request.didRespond).to.be.true);
expect(resolved).to.have.members([1, 2, 3]);
});
});
51 changes: 47 additions & 4 deletions packages/@pollyjs/adapter/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,15 +91,16 @@ export default class Adapter {
const pollyRequest = this.polly.registerRequest(request);

await pollyRequest.setup();
await this.onRequest(pollyRequest);

try {
const response = await this[REQUEST_HANDLER](pollyRequest);
const result = await this[REQUEST_HANDLER](pollyRequest);

pollyRequest.promise.resolve(response);
await this.onRequestFinished(pollyRequest, result);

return response;
return result;
} catch (error) {
pollyRequest.promise.reject(error);
await this.onRequestFailed(pollyRequest, error);
throw error;
}
}
Expand Down Expand Up @@ -213,6 +214,7 @@ export default class Adapter {
);
}

/* Required Hooks */
onConnect() {
this.assert('Must implement the `onConnect` hook.', false);
}
Expand All @@ -221,19 +223,60 @@ export default class Adapter {
this.assert('Must implement the `onDisconnect` hook.', false);
}

/**
* @param {PollyRequest} pollyRequest
* @returns {*}
*/
onRecord() {
this.assert('Must implement the `onRecord` hook.', false);
}

/**
* @param {PollyRequest} pollyRequest
* @param {Object} normalizedResponse The normalized response generated from the recording entry
* @param {Object} recordingEntry The entire recording entry
* @returns {*}
*/
onReplay() {
this.assert('Must implement the `onReplay` hook.', false);
}

/**
* @param {PollyRequest} pollyRequest
* @param {PollyResponse} response
* @returns {*}
*/
onIntercept() {
this.assert('Must implement the `onIntercept` hook.', false);
}

/**
* @param {PollyRequest} pollyRequest
* @returns {*}
*/
onPassthrough() {
this.assert('Must implement the `onPassthrough` hook.', false);
}

/* Other Hooks */
/**
* @param {PollyRequest} pollyRequest
*/
onRequest() {}

/**
* @param {PollyRequest} pollyRequest
* @param {*} result The returned result value from the request handler
*/
onRequestFinished(pollyRequest, result) {
pollyRequest.promise.resolve(result);
}

/**
* @param {PollyRequest} pollyRequest
* @param {Error} error
*/
onRequestFailed(pollyRequest, error) {
pollyRequest.promise.reject(error);
}
}
6 changes: 1 addition & 5 deletions packages/@pollyjs/core/src/-private/request.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,7 @@ export default class PollyRequest extends HTTPBase {
get absoluteUrl() {
const { url } = this;

if (!isAbsoluteUrl(url)) {
return new URL(url).href;
}

return url;
return isAbsoluteUrl(url) ? url : new URL(url).href;
}

get protocol() {
Expand Down
33 changes: 18 additions & 15 deletions tests/integration/adapter-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,29 +100,32 @@ export default function adapterTests() {
expect(responseCalled).to.be.true;
});

it('should have responses after flushing', async function() {
it('should have resolved requests after flushing', async function() {
// The puppeteer adapter has its own implementation of this test
if (this.polly.adapters.has('puppeteer')) {
this.skip();
}

const { server } = this.polly;
const requests = [];
const resolved = [];

server.get(this.recordUrl()).intercept(async (req, res) => {
requests.push(req);
await server.timeout(5 * requests.length);
res.sendStatus(200);
});

this.fetchRecord();
this.fetchRecord();
this.fetchRecord();
server
.get(this.recordUrl())
.intercept(async (req, res) => {
await server.timeout(5);
res.sendStatus(200);
})
.on('request', req => requests.push(req));

// Wait for all requests to be made
// (Puppeteer adapter takes some time to make the actual request)
while (requests.length !== 3) {
await server.timeout(5);
}
this.fetchRecord().then(() => resolved.push(1));
this.fetchRecord().then(() => resolved.push(2));
this.fetchRecord().then(() => resolved.push(3));

await this.polly.flush();

expect(requests).to.have.lengthOf(3);
requests.forEach(request => expect(request.didRespond).to.be.true);
expect(resolved).to.have.members([1, 2, 3]);
});
}