Add support for :disallow-mock metadata.

[technomancy]

One common mistake when using bond is to mock the wrong function; we have had a lot of situations where we accidentally mocked a function which was doing spec checks, and thus those specs were not checked during the test runs.

This patch allows you to mark a function as :disallow-mock which will prevent bond from operating on it.

[neeasade]

we have had a lot of situations where we accidentally mocked a function which was doing spec checks, and thus those specs were not checked during the test runs.

~note: The motivation for this PR overlaps with https://github.com/circleci/bond/issues/46 a bit~ disregard, the example in the PR is clarifying

[calvis]

i feeel like this is an antipattern, can you say more about how you ended up in a the situation where you've mocked the wrong function? could one not misplace the metadata for the same reasons?

this kind of unexpected behavior (am i checking specs?) is one of the core fundamental drawbacks to mocking - mocked tests are fragile in the face of changes to the code being tested. you constantly have to keep track of what is really being tested. by mocking you are making a tradeoff between simplicity in tests for complexity in maintenance.

we could try to mitigate this exact scenario but i have doubts about how much it would help - it would prevent mocking in the scenario that was documented but it doesn't make clear why the important function can't be mocked by the future maintainer and someone could simply move the spec check to a helper and suddenly the protection has been lost

[technomancy]

can you say more about how you ended up in a the situation where you've mocked the wrong function?

We have a situation where we distribute a client for one of our microservices to use in many other codebases that communicates over GRPC to our service. Because of how weird and twisty our internal support GRPC library is, it's very difficult to identify the correct function to mock when you're writing tests for code that talks to our service. (The correct place is to mock exactly before the GRPC request hits the network.) It's also very difficult to mock this function correctly in a way that can discriminate between multiple different functions that all go thru the same GRPC plumbing so that endpoint A returns mock-data A and endpoint B returns mock-data B.

Frequently people instead make the mistake of mocking the outermost public function of the client. We have painstakingly added spec checks to the client so that tests which use incorrect mock data will cause exceptions, but if people mock the outermost function, those spec checks will never run, and the tests will pass even though they use incorrect data. As part of our client library we include a specialized mock namespace which can give you the spec checks you need, but it's easy to forget and just use bond/with-stub instead because that's convenient.

could one not misplace the metadata for the same reasons?

The key difference here is that the people who are writing the client library have a much better understanding of which function is mocked than the people consuming the library.

[gordonsyme]

The key difference here is that the people who are writing the client library have a much better understanding of which function is mocked than the people consuming the library.

Is it viable to provide a suitable mock client as part of the library?

[calvis]

As part of our client library we include a specialized mock namespace which can give you the spec checks you need, but it's easy to forget and just use bond/with-stub instead because that's convenient.

i feel like this is a human problem and a human-oriented solution is probably best, instead of trying to change their behavior with a guardrail. is there friction that prevents those engineers from using the specialized namespace? is it widely known that it is available? does it cover all use cases?

[technomancy]

Is it viable to provide a suitable mock client as part of the library?

The mock is included in the library already; the problem is it's overlooked. The intent of this PR is to get people to use the mock that already exists.

i feel like this is a human problem and a human-oriented solution is probably best, instead of trying to change their behavior with a guardrail.

I feel like with the rate our company is hiring this problem will only get worse with time. On our team only 2 of the 8 engineers have been on the team for more than 9 months. Obviously we're doing all we can to train people and bring them up to speed on things, but the human-oriented solution does not scale to the level of growth we have seen over the last year. The human-oriented solution is "don't grow your team at too rapid of pace" but that's not my call to make, so I need to fall back to a more mechanical form of support.

[technomancy]

I think we can drop this now.