Skip to content

drpcmux: add tests for server interceptors #14

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

Merged
merged 1 commit into from
Jul 31, 2025

Conversation

shubhamdhama
Copy link

@shubhamdhama shubhamdhama commented Jul 20, 2025

This commit adds tests for the server interceptors and RPC handling:

  • Tests for unary and streaming server interceptors
  • Validation of correct interceptor chaining and execution order
  • Error handling and propagation through interceptor chains
  • Edge cases with empty interceptor chains

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds comprehensive test coverage for server interceptors in the drpcmux package. The purpose is to validate the correct behavior of interceptor chaining, execution order, and error handling for both unary and streaming RPC types.

  • Added interceptor chaining tests with execution order validation
  • Implemented edge case handling for empty interceptor chains and error propagation
  • Enhanced HandleRPC functionality to support proper interceptor selection based on RPC type

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
drpcmux/interceptor_test.go Tests for interceptor chaining, execution order, and error handling with mock implementations
drpcmux/handle_rpc_test.go Comprehensive tests for HandleRPC method with various interceptor scenarios and RPC types
drpcmux/handle_rpc.go Refactored HandleRPC method to properly select interceptors based on RPC type and extract message receiving logic
Comments suppressed due to low confidence (3)

drpcmux/handle_rpc_test.go:24

  • [nitpick] The field name 't' is not descriptive. Consider renaming it to 'testingT' or 'test' for better clarity.
	t              *testing.T

drpcmux/handle_rpc_test.go:31

  • [nitpick] The method name 'mockMethod' is generic and doesn't reflect its actual purpose. Consider renaming to 'handleUnaryRequest' or similar to clarify its role.
func (m *mockRPCImpl) mockMethod(_ context.Context, in *mockMessage) (*mockMessage, error) {

drpcmux/handle_rpc_test.go:40

  • [nitpick] The method name 'mockStreamMethod' is generic and doesn't reflect its actual purpose. Consider renaming to 'handleStreamRequest' or similar to clarify its role.
func (m *mockRPCImpl) mockStreamMethod(_ drpc.Stream) error {

Copy link

@cthumuluru-crdb cthumuluru-crdb left a comment

Choose a reason for hiding this comment

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

LGTM. Eventually, lets change these tests to use generated protobuf code.

This commit adds tests for the server interceptors and RPC handling:

- Tests for unary and streaming server interceptors
- Validation of correct interceptor chaining and execution order
- Error handling and propagation through interceptor chains
- Edge cases with empty interceptor chains
@shubhamdhama shubhamdhama force-pushed the add-drpc-srv-inc-tests branch from 689f0dd to f8c3b09 Compare July 31, 2025 14:18
@shubhamdhama shubhamdhama merged commit 81273aa into cockroachdb:main Jul 31, 2025
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