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

Provide an ergonomic way to test interceptors #740

Closed
advdv opened this issue May 21, 2024 · 2 comments
Closed

Provide an ergonomic way to test interceptors #740

advdv opened this issue May 21, 2024 · 2 comments
Labels
enhancement New feature or request

Comments

@advdv
Copy link

advdv commented May 21, 2024

Is your feature request related to a problem? Please describe.
It seems that it is currently required to start a http server to test middleware that is dependent on the req.Spec or req.Peer. This in turn makes it very hard to test contextual values (because now they need to be carried from the client to the sever, over the wire). I propose that it should be easy to:

  • Test interceptors that have conditional logic on the .Spec member (e.g: isClient, isServer, authorization based on .Procedure)
  • Test interceptors that have conditional logic on the Peer member (e.g: authorization based on network addresses)
  • Test interceptors that have conditional logic what is based on the context (e.g: use a JWT set earlier in the middleware chain)

Ideally it would be something like this

var interceptor := NewInterceptor(...) // connect.UnaryInterceptorFunc

// setup the request
req := connect.NewRequest(&rpcv1.WhoAmIRequest{})
req.Spec = connect.Spec{} // set the spec for this test

// add something to the context
ctx :=  context.WithValue(ctx, "some jwt")

// test the middleware
resp, err := interceptor.WrapUnary(func(ctx context.Context, ar connect.AnyRequest) (connect.AnyResponse, error) {

    return connect.NewResponse(&rpcv1.WhoAmIResponse{}), nil
})(ctx, req)

Describe alternatives you've considered
It seems the only alternatives art:

  1. to start a httptest.Server. But then how do I easily get my contextual values in the handler. And how to I set Peer values to the values I need to test in this case?
  2. Another solution is to just use connect.NewRequest() and call it in-memory. But then I can't specify the .Spec or .Peer
  3. Another issue proposes to use an in-memory server, this library has it, but doesn't expose it. instead asking to do it ourselves.
  4. Another option is to write a custom Request type that embed the AnyRequest. This new request can expose Spec and Peer. This is the most elegant solution but I still wonder if every library should have to do this. Can't we have a connecttest package that mirrors the httptest in the standard library for something like this.

None of this is ideal and it prevents people from creating well-testing interceptors that the ecosystem can benefit from.

@advdv advdv added the enhancement New feature or request label May 21, 2024
@emcfarlane
Copy link
Contributor

The current recommend workaround to test interceptors in isolation is to create the AnyRequest and set the required fields. See this discussion: #708 (comment)

Also see this related issue: #719 which is blocked on API discussions.

@advdv
Copy link
Author

advdv commented May 22, 2024

Right, then this can be considered a duplicate (sorry did search).

@advdv advdv closed this as completed May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants