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

feat(pubsub/pstest): add channel to support user-defined publish responses #4251

Merged
merged 3 commits into from Jun 15, 2021

Conversation

@hongalex
Copy link
Member

@hongalex hongalex commented Jun 12, 2021

Taken from the Java fake publisher service, these changes improves testing by allowing users to specify PublishResponses that the fake server will return for Publish calls.

This is implemented with a buffered publishResponses channel, that users can add to by disabling automatic publish responses (the current behavior) and calling AddPublishResponse.

srv.SetAutoPublishResponse(false)
srv.AddPublishResponse(&pb.PublishResponse{
  MessageIds: []string{"1"}
}, nil)
srv.Publish("projects/p/topics/t", []byte("some-message"), nil)

Lastly, the default buffered channel size is 100 (an arbitrary number suitable for testing). If the size needs to be increased, or you want to reset the publish queue, call ResetPublishResponses.

The changes in this PR will be used to help test publisher flow control.

@hongalex hongalex requested a review from shollyman Jun 14, 2021
Copy link
Contributor

@shollyman shollyman left a comment

LGTM, with a minor caveat which may or may not be relevant given my lack of experience with the p/s fake.

Loading

@@ -613,6 +658,15 @@ func (s *GServer) Publish(_ context.Context, req *pb.PublishRequest) (*pb.Publis
if top == nil {
return nil, status.Errorf(codes.NotFound, "topic %q", req.Topic)
}

if !s.autoPublishResponse {
r := <-s.publishResponses
Copy link
Contributor

@shollyman shollyman Jun 15, 2021

Choose a reason for hiding this comment

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

Unclear if it matters, but do you want blocking behavior here, or should it emit an error in the case of some kind of timeout via a select loop or similar?

Loading

Copy link
Member Author

@hongalex hongalex Jun 15, 2021

Choose a reason for hiding this comment

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

I want blocking behavior, but could be convinced to add a timeout/error in the future.

Loading

@hongalex hongalex merged commit e1304f4 into googleapis:master Jun 15, 2021
4 checks passed
Loading
@hongalex hongalex deleted the pubsub-fake-publish-queue branch Jun 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants