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

Add 3 helper APIs on top of MITM callbacks #119

Merged
merged 5 commits into from
Jul 11, 2024
Merged

Add 3 helper APIs on top of MITM callbacks #119

merged 5 commits into from
Jul 11, 2024

Conversation

kegsay
Copy link
Member

@kegsay kegsay commented Jul 10, 2024

Part of #68

These helper APIs all have proven needs in existing tests, and this commit converts at least 1 test for each type of helper API to demonstrate how they can be used.

  • callback.SendError: the simplest of the new APIs. This sends an HTTP error instead of the response a configurable number of times, after which point it passes the response through. Useful for testing that clients retry endpoints.
  • callback.PassiveChannel: this sniffs callback data and sends it on a buffered (non-blocking) or unbuffered (blocking) channel. Useful for testing that the client sent a request.
  • callback.ActiveChannel: this is a passive channel but it also allows responses to be sent back for each intercepted callback. As such, it is always an unbuffered (blocking) channel. Useful for performing an action when the test sees a certain callback.

Channels exist to reduce the amount of concurrent code that needs to be written for each test. It does this by:

  • providing synchronisation points,
  • with built-in timeouts,
  • and handles common channel gotchas (e.g sending on a closed channel panics)

Tests can just using normal Go chan, but then they will need to reimplement these things for each test.

A good example of how this can simplify code is to look at how this PR changes testUnprocessedToDeviceMessagesArentLostOnRestartJS which closes the client when it detects a /sync response. Previously, we needed 2 additional goroutines and multiple waiters to synchronise between them. Using a single ActiveChannel, we only need 1 additional goroutine (to call StartSyncing() as that blocks) and that's it. The channel manages the synchronisation between the callback goroutine and the test goroutine, meaning the test goroutine itself can manage checking the callback data, whereas this had to be done in the callback goroutine before. It is always preferable for the main test goroutine (the goroutine running TestWhatever) to deal with data, as that means the test needs no additional synchronisation primitives to control access to that data.

These helper APIs all have proven needs in existing tests, and this
commit converts at least 1 test for each type of helper API to demonstrate
how they can be used.

- `callback.SendError`: this sends an HTTP error instead of the response
  a configurable number of times, after which point it passes the response
  through. Useful for testing endpoints retry.
- `callback.PassiveChannel`: this sniffs callback data and sends it on
  a buffered (non-blocking) or unbuffered (blocking) channel. Useful for
  testing that the client sent a request.
- `callback.ActiveChannel`: this is a passive channel but it also allows
  responses to be sent back for each intercepted callback. As such, it
  is always an unbuffered (blocking) channel. Useful for performing an
  action when the test sees a certain callback.
@kegsay kegsay requested a review from andybalaam July 10, 2024 14:57
Copy link
Member

@andybalaam andybalaam left a comment

Choose a reason for hiding this comment

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

Looks good, and makes sense as far as I understand it. Couple of questions and tiny suggestions.

internal/deploy/callback/callback_addon.go Outdated Show resolved Hide resolved
internal/deploy/callback/callback_addon.go Outdated Show resolved Hide resolved
internal/deploy/callback/callback_addon.go Outdated Show resolved Hide resolved
internal/deploy/callback/callback_addon.go Outdated Show resolved Hide resolved
kegsay and others added 3 commits July 11, 2024 09:27
@kegsay kegsay merged commit af8dbb1 into main Jul 11, 2024
4 checks passed
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

Successfully merging this pull request may close these issues.

2 participants