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

Nock setup file #19

Closed
mikicho opened this issue Nov 7, 2020 · 10 comments
Closed

Nock setup file #19

mikicho opened this issue Nov 7, 2020 · 10 comments

Comments

@mikicho
Copy link
Contributor

mikicho commented Nov 7, 2020

I'm new to nock.
Is it common to create a set of nocks in the global setup and override them if possible on the local tests?
For example for URLs that get called in a lot of requests (authorization, permissions, sqs...)

@goldbergyoni
Copy link
Contributor

Technically - global-setup happens in a different process so it's applicable for cross-process setup things like DB. Nock patches Node libraries hence if you try to configure it within global-setup it won't work

Fundamentally - As a principle, tests should be scoped locally as much as possible. When I read a test, I hope to see everything inside. Sometimes it's not feasible, so the next place I'd search for config in the nearest test hooks on the same file. This way the test reader search experience is predictable. When having some repeating 3rd party service behaviour, things that are repeated across test files (e.g. get user info from the user Microservice), it makes sense to define the behaviour in a helper file but call that from each test file beforeEach hook - This way the reader easily finds out all the side effects that affect the tests. In closing - Everything that can change the test is defined within the test or at the nearest hook

Makes sense? Better alternatives?

@mikicho
Copy link
Contributor Author

mikicho commented Nov 8, 2020

Thanks for the detailed answer.
why beforeEach and not beforeAll with persist?

@goldbergyoni
Copy link
Contributor

My go-to pattern is to always clean-up and re-define beforeEach test so I know that no state is preserved between the tests. For example, consider defining some test double in beforeAll, then some test modifies this double, later tests might fail because of the global state/mock. It's always good to start with a clean slate.

But what is the performance penalty of re-applying nock before every test? I never tested, I guess this is why we have this repo:)

@mikicho
Copy link
Contributor Author

mikicho commented Nov 11, 2020

But what is the performance penalty of re-applying nock before every test?

I tested it when open this issue.
If I remember correctly, on 100 nocks the implications start to be annoying (about 10ms per test). but 100 nocks are a lot and I doubt if anyone would need to apply so many nocks for every test.

So if I summarize it up to a best practice: create a nock helper file that contains a common pattern and call it before each?

@goldbergyoni
Copy link
Contributor

Performance - Do you know what is the cost if beforeEach defines 3 nocks?

Best practice - I would suggest that:

Golden rule - A test visitor will always find the failure reason within the test or at the worst case in the nearest hook.

Practically:

  • If the intercepted request affects the test result (e.g. user service returns that 404) - It must be part of the test!
  • If the intercepted request is needed for all the tests in the file - Put it in beforeEach
  • If there are many intercepted requests and the definition becomes verbose - Extract it to a helper file that is being called by the test file hook

Makes sense?

@mikicho
Copy link
Contributor Author

mikicho commented Nov 13, 2020

Performance - Do you know what is the cost if beforeEach defines 3 nocks?

I'd say it's roughly about 1ms for each nock: so 3 nocks = 3 ms, 100 nocks = 100ms
https://repl.it/join/xhqelaqg-solomomi

Golden rule - A test visitor will always find the failure reason within the test or at the worst case in the nearest hook.

I think this is a pretty confusing one.. the golden rule has to write done next/after the practical rules you list in your comment.

@goldbergyoni
Copy link
Contributor

All of these performance checks are great, we can explain every decision:)

So the 3 rules do make sense? I just think that it's not related to nock rather to everything. A test reader should inspect no more than 7 lines of test or the hook above it. Not inspect base class, 50 tests in the middle, etc

@mikicho
Copy link
Contributor Author

mikicho commented Nov 13, 2020

I agree and understand.
My only concern is about complex nocks we need cross files but maybe they are rare

Let's move on and see if those rules are enough.

@goldbergyoni
Copy link
Contributor

They are absolutely not rare. In that case, one can do the wrong thing:

  • Put all of them in an external file

Or the better option:

  • Put in external but let the hook invoke this and signal to the reader about this existing effect
  • Those who affects the test result will appear inside the tests

@mikicho
Copy link
Contributor Author

mikicho commented Nov 14, 2020

@goldbergyoni a potential problem with an external file approach:
what if I have the following endpoint:

https://example.com/resource

And I nock it in an external file:

nock('http://www.example.com')
  .get('/resource')
  .reply(200, 'domain matched');

and in a specific test I want to override this interceptor and return 500 instead:

test('example service should return 500', () => {
const scope = nock('http://www.example.com')
  .get('/resource')
  .reply(500, 'domain matched');

expect(scope.isDone()).toBe(true); // failed because the nock in the external file get called.
});

I think we can solve this by expose the external file scopes to the test like:
common-nock.js

module.exports = {
  exampleScope: nock('http://www.example.com')
  .get('/resource')
  .reply(200, 'domain matched'),
}

and then remove the interceptor before the test (nock.removeInterceptor?)
WDYT? do you have a better technic?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants