cachex icon indicating copy to clipboard operation
cachex copied to clipboard

Add @callback's to better facilitate testing

Open fireproofsocks opened this issue 5 years ago • 2 comments

When using the mox package to create dynamic mocks to simulate function responses, it is required to have @callback's defined for each function. Mox doesn't care where these @callbacks are defined, so in a pinch, we have created our own module which defines @callback specs that mirror the Cachex function @spec's.

However, I think it would be preferable if Cachex defined its own contract as a behaviour with its own@callbacks. The advantage of this would go beyond tests and mocks, it could extend to other implementations and variations.

Perhaps ExAws.Behaviour is a useful example of how another library defined a behaviour.

Thanks for your consideration; I can put together a pull request if you feel this contribution would be helpful.

fireproofsocks avatar Jun 09 '20 03:06 fireproofsocks

If you wanted to take the time to file a PR for this, I don't think I have any reason not to include it as long as it doesn't break any existing contracts somehow.

Fire away!

whitfin avatar May 25 '21 18:05 whitfin

Adding a few bits: if I understood well, Mox by default works by setting per-process expectations. It also appears that Cachex.fetch "do" block runs in a separate process. This means that by default, Cachex tests won't work with Mox.

One would have to use explicit allowances (but this requires to modify the caching code itself I believe), or go "global" (https://hexdocs.pm/mox/Mox.html#module-multi-process-collaboration).

I believe a behaviour could help here. That said it will make tests work quite differently from production, and I'm not sure ultimately I would want to "bypass Cachex" inside my tests, if caching is an essential code path.

Just adding a bit of data to the discussion, I do not have a clear picture on all this yet.

thbar avatar Jun 10 '21 16:06 thbar

I just wanted to circle back to this; I personally avoid mocking whenever possible. As such, I don't have a clear view of what is required and/or would be helpful for this use case - if anyone can give me more information or wants to suggest some changes, I'm happy to look into it though!

Given this thread has had little action for several years, I'm going to close it for now as it seems to be low priority - but please don't hesitate to re-open if you're landing here in the future and think we need to pick it back up!

whitfin avatar Jan 04 '24 13:01 whitfin

@whitfin thanks for asking! Ultimately here is what I did:

  • https://github.com/etalab/transport-site/blob/master/apps/transport/lib/transport/cache.ex I created a behaviour that wraps Cachex, with a default implementation which is used in the real app
  • https://github.com/etalab/transport-site/blob/14d9d74492f5b308f46ab49ed9b5732bb80203a7/apps/transport/lib/transport/cache.ex#L88 during test, I use a no-op implementation which makes sure we do not run into troubles related to "out-of-process expectations"

This works good enough so far, and I've used this strategy whenever a library does not propose a behaviour, with good success.

I would say this can be probably postponed given the lack of advertised interest, and the fact that "wrapping on top" works for most use-cases.

thbar avatar Jan 04 '24 13:01 thbar

@thbar so would support for this within Cachex be as simple as adding @callback for every function signature in the main Cachex API?

I guess I'm surprised there's no way to do this automatically within these mocking tools, it feels like you should be able to generate it from the exported functions? Just feels a little redundant, maybe? Happy to add it though if it helps!

Is there some way I can replace @spec with @callback? Or would I need both?

whitfin avatar Jan 04 '24 14:01 whitfin