ijpiantanida / talkback

A simple HTTP proxy that records and playbacks requests
MIT License
284 stars 41 forks source link

Feature Request: Add a RecordMode.ALWAYS option #26

Open tartale opened 5 years ago

tartale commented 5 years ago

The new RecordMode is great! I especially like the ability to specify it per-request.

An additional use case for the new RecordMode would be an ALWAYS mode. In other words, a new tape is created with a request and response, whether or not a matching tape exists.

An example where this could be useful is this:

If I use RecordMode.New, the 404 will be returned forever, because the initial tape was made from the bad response, and then re-used

If I use RecordMode.Overwrite, I lose the middle tape, which had the good 'info'

If I use RecordMode.Disabled, I don't get a tape at all

With RecordMode.Always, I'll get all three tapes - then I can keep the tape I like, change over to RecordMode.Disabled, and re-run

Thoughts/opinions? I'd be happy to do a PR on this if you're interested and don't have time

ijpiantanida commented 5 years ago

I see how this mode can be useful, but I'm doubtful about simply creating multiple tapes that match the same request, since that could easily slip through and lead to unwanted and hard to debug confusion.

I'm not sure what a good solution could be though. Even telling that 2 tapes are for the same request is not straightforward since the matching can be dynamic through some of the options. If you want, you can create a PR to add the ALWAYS mode, and I'll try to think of and add a good way of alerting users that more than one taped matched a request.

vgrigoruk commented 5 years ago

I'm using talkback to test a frontend SPA in isolation from API backend with selenium tests and I'm facing the same situation. SPA sends the same http request twice during a test and the actual response from API is different in both cases. Having something like RecordMode.Always could theoretically help. I've started looking into talkback code to figure out how much changes are needed to support RecordMode.Always. I guess if multiple tapes are matching the request, I need to return them in chronological order (based on meta.createdAt). Do you see any potential issue with this approach?

ijpiantanida commented 5 years ago

The problem I see with this approach is: Requests 1 and 2 come, so talkback returns those 2 tapes in the original order. Now comes a 3rd request, because let's say, you're running another test that goes through the same flow. How does talkback know that the cycle started again? What if one test makes 2 requests and another one makes 3? What if one tests is broken and it skips a request? How do you make sure that only that test breaks without affecting the rest of the suite?

These are all problems related to the fact that talkback isn't aware of a "test context" and that all tests share the same collection of tapes, and that you're trying to mock something stateful with something stateless.

The solution that other libraries take for this, is to make each test have its own set of tapes, and make the mocking server stateful. This comes at the cost of having to repeat tapes and difficulties with parallelism, so I'm not sure it's a tradeoff worth taking.

cbrunnkvist commented 4 years ago

Either way "ALWAYS" doesn't clearly communicate what makes it different from NEW vs OVERWRITE (-both are currently much less ambiguous in comparison).

Perhaps what we mean is a "multiple" or "loop"-variation to each of NEW/OVERWRITE, which could use a logic similar to this:

(note: involving non-suffixed names, in order to keep them backward compatible with existing recordings.)