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 all commits
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
14 changes: 14 additions & 0 deletions docs/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -251,3 +251,17 @@ __Example__
```js
polly.disconnect();
```

### flush

Returns a Promise that resolves once all requests handled by Polly have resolved.

| Param | Type | Description |
| --- | --- | --- |
| Returns | `Promise` |   |

__Example__

```js
await polly.flush();
```
6 changes: 4 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,16 @@
"clean": "lerna run clean --parallel",
"bootstrap": "lerna bootstrap",
"format": "lerna run format",
"lint": "lerna exec --bail=false -- yarn run lint",
"lint": "lerna exec --bail=false --stream --no-prefix -- yarn run lint",
"pretest": "yarn server:build",
"pretest:ci": "yarn pretest",
"test": "testem",
"test:ci": "testem ci",
"test:build": "lerna run test:build --parallel",
"test:clean": "rimraf packages/@pollyjs/*/build",
"test:ember": "lerna run test --scope=@pollyjs/ember",
"test:node": "mocha --opts tests/mocha.opts",
"test:jest": "jest",
"test:ember": "lerna run test --stream --no-prefix --scope=@pollyjs/ember",
"server:build": "yarn build --scope=@pollyjs/node-server --scope=@pollyjs/utils",
"docs:serve": "docsify serve ./docs",
"docs:publish": "gh-pages --dist docs --dotfiles --message 'chore: Publish docs'",
Expand Down
55 changes: 50 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,68 @@ 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(request.response());
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]);
});
});
65 changes: 59 additions & 6 deletions packages/@pollyjs/adapter/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,17 +87,28 @@ export default class Adapter {
}
}

handleRequest() {
return this[REQUEST_HANDLER](...arguments);
async handleRequest(request) {
const pollyRequest = this.polly.registerRequest(request);

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

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

await this.onRequestFinished(pollyRequest, result);

return result;
} catch (error) {
await this.onRequestFailed(pollyRequest, error);
throw error;
}
}

async [REQUEST_HANDLER](request) {
async [REQUEST_HANDLER](pollyRequest) {
const { mode } = this.polly;
const pollyRequest = this.polly.registerRequest(request);
let interceptor;

await pollyRequest.setup();

if (pollyRequest.shouldIntercept) {
interceptor = new Interceptor();
const response = await this.intercept(pollyRequest, interceptor);
Expand Down Expand Up @@ -203,6 +214,7 @@ export default class Adapter {
);
}

/* Required Hooks */
onConnect() {
this.assert('Must implement the `onConnect` hook.', false);
}
Expand All @@ -211,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);
}
}
8 changes: 3 additions & 5 deletions packages/@pollyjs/core/src/-private/request.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import PollyResponse from './response';
import NormalizeRequest from '../utils/normalize-request';
import parseUrl from '../utils/parse-url';
import serializeRequestBody from '../utils/serialize-request-body';
import DeferredPromise from '../utils/deferred-promise';
import isAbsoluteUrl from 'is-absolute-url';
import { assert, timestamp } from '@pollyjs/utils';
import HTTPBase from './http-base';
Expand All @@ -29,6 +30,7 @@ export default class PollyRequest extends HTTPBase {
this.recordingName = polly.recordingName;
this.recordingId = polly.recordingId;
this.requestArguments = freeze(request.requestArguments || []);
this.promise = new DeferredPromise();
this[POLLY] = polly;

/*
Expand All @@ -54,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
10 changes: 10 additions & 0 deletions packages/@pollyjs/core/src/polly.js
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,16 @@ export default class Polly {
}
}

async flush() {
const NOOP = () => {};

await Promise.all(
// The NOOP is there to handle both a resolved and rejected promise
// to ensure the promise resolves regardless of the outcome.
this._requests.map(r => Promise.resolve(r.promise).then(NOOP, NOOP))
);
}

/**
* @param {String|Function} nameOrFactory
* @public
Expand Down
16 changes: 16 additions & 0 deletions packages/@pollyjs/core/src/utils/deferred-promise.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/**
* Create a deferred promise with `resolve` and `reject` methods.
*/
export default class DeferredPromise extends Promise {
constructor() {
let resolve, reject;

super((_resolve, _reject) => {
resolve = _resolve;
reject = _reject;
});

this.resolve = resolve;
this.reject = reject;
}
}
29 changes: 29 additions & 0 deletions packages/@pollyjs/core/tests/unit/utils/deferred-promise-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import DeferredPromise from '../../../src/utils/deferred-promise';

describe('Unit | Utils | DeferredPromise', function() {
it('should exist', function() {
expect(DeferredPromise).to.be.a('function');
expect(new DeferredPromise().resolve).to.be.a('function');
expect(new DeferredPromise().reject).to.be.a('function');
});

it('should resolve when calling .resolve()', async function() {
const promise = new DeferredPromise();

promise.resolve(42);
expect(await promise).to.equal(42);
});

it('should reject when calling .reject()', async function() {
const promise = new DeferredPromise();

promise.reject(new Error('42'));

try {
await promise;
} catch (error) {
expect(error).to.be.an('error');
expect(error.message).to.equal('42');
}
});
});
7 changes: 3 additions & 4 deletions testem.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ module.exports = {
'./tests/*',
'./tests/!(recordings)/**/*',
'./packages/@pollyjs/*/src/**/*',
'./packages/@pollyjs/*/tests/*/*'
'./packages/@pollyjs/*/tests/**/*'
],
serve_files: ['./packages/@pollyjs/*/build/browser/*.js'],
browser_args: {
Expand All @@ -33,12 +33,11 @@ module.exports = {
middleware: [attachMiddleware],
launchers: {
Node: {
command:
'mocha ./packages/@pollyjs/*/build/node/*.js --ui bdd --reporter tap --require tests/node-setup.js',
command: 'yarn test:node --reporter tap',
protocol: 'tap'
},
Jest: {
command: 'jest',
command: 'yarn test:jest',
protocol: 'tap'
},
Ember: {
Expand Down
Loading