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: Server level configuration #80

Merged
merged 9 commits into from
Sep 12, 2018
Merged

feat: Server level configuration #80

merged 9 commits into from
Sep 12, 2018

Conversation

offirgolan
Copy link
Collaborator

@offirgolan offirgolan commented Jul 30, 2018

Proposed Changes

intercept

Intercept can now be registered on the middleware level. If multiple intercept handlers have been
registered, each handler will be called in the order in which it was registered.

Example

server
  .any('/session')
  .intercept((req, res) => res.sendStatus(200));

passthrough

Passthrough can now be registered on the middleware level. It also now accepts a boolean which allows the user to explicitly enable or disable it.

Param Type Description
passthrough boolean Enable or disable the passthrough. Defaults to true

Example

server
  .any('/session')
  .passthrough();

server
  .get('/session/1')
  .passthrough(false);

recordingName new

Override the recording name for the given route. This allows for grouping common
requests to share a single recording which can drastically help de-clutter test
recordings.

For example, if your tests always make a /users or /session call, instead of
having each of those requests be recorded for every single test, you can use
this to create a common recording file for them.

Param Type Description
recordingName String Name of the recording to store the recordings under.

Example

server
  .any('/session')
  .recordingName('User Session');

server
  .get('/users/:id')
  .recordingName('User Data');

server
  .get('/users/1')
  .recordingName(); /* Fallback to the polly instance's recording name */

configure new

Override configuration options for the given route. All matching middleware and route level configs are merged together and the overrides are applied to the current
Polly instance's config.

The following options are not supported to be overriden via the server API:
mode, adapters, adapterOptions, persister, persisterOptions

Param Type Description
config Object Configuration object

Example

server
  .any('/session')
  .configure({ recordFailedRequests: true });

server
  .get('/users/:id')
  .configure({ timing: Timing.relative(3.0) });

server
  .get('/users/1')
  .configure({ logging: true });

@offirgolan offirgolan added the enhancement ✨ New feature or request label Jul 30, 2018
@offirgolan offirgolan requested a review from jasonmit July 30, 2018 05:07
@jasonmit
Copy link
Contributor

jasonmit commented Aug 3, 2018

I'm worried req.configure() will be confused with polly.configure() as I imagine we can only support a subset of options here.

It's unclear when one would use the request-level configuration over the server/route level. Should we simplify and only support one for now until there is a clear use-case?

@offirgolan offirgolan changed the title [WIP] feat: Server & request level configuration [WIP] feat: Server level configuration Aug 30, 2018
@offirgolan offirgolan changed the title [WIP] feat: Server level configuration feat: Server level configuration Sep 10, 2018
].forEach(key =>
expect(() => server.any().configure({ [key]: 'foo' })).to.throw(
Error,
/Invalid configuration option/
Copy link
Contributor

Choose a reason for hiding this comment

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

Error could include these options not being allowed to be configure through the new config API vs invalid configuration options since they're technically valid.

Copy link
Collaborator Author

@offirgolan offirgolan Sep 11, 2018

Choose a reason for hiding this comment

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

The entire error message is:

`Invalid configuration option provided. The "${key}" option cannot be overridden.`

Do you have anything else in mind?

@@ -3,7 +3,7 @@ import Timing from '../../../src/utils/timing';
function fixedTest(ms) {
it(`should handle ${ms}ms`, async function() {
// Fail the test if it exceeds ms + 10ms buffer
this.timeout(ms + 10);
this.timeout(ms + 50);
Copy link
Contributor

Choose a reason for hiding this comment

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

thanks!

@offirgolan offirgolan merged commit 0f32d9b into master Sep 12, 2018
@offirgolan offirgolan deleted the request-level-config branch September 12, 2018 02:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement ✨ New feature or request 📦 core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants