golang / mock

GoMock is a mocking framework for the Go programming language.
Apache License 2.0
9.32k stars 607 forks source link

handle golang.org/x/net/context & context import collisions with source mode #156

Closed Rajczyk closed 4 years ago

Rajczyk commented 6 years ago

So I've just been testing mock with protocol buffer generated code on a simple .proto file

syntax = "proto3";

package pb;

// A generic empty message
message Empty {

}

// The Add service definition.
service Random {
    // Gets a random number.
    rpc Get (Empty) returns (stream GetReply) {}
}

// The get response contains the random number.
message GetReply {
    int64 v = 1;
    string err = 2;
}

To generate the above you can do

Install proto3 https://github.com/google/protobuf/releases mv protoc/bin/protoc /usr/local/bin/protoc

Update protoc Go bindings via go get -u github.com/golang/protobuf/{proto,protoc-gen-go}

protoc random.pb --go_out=plugins=grpc:.;

The important part to notice of this definition is the usage of streams which causes an interface of interfaces to get generated by protoc

type Random_GetClient interface { Recv() (*GetReply, error) grpc.ClientStream }

When using mockgen on this file I get "imported package collision: context imported twice". Now I understand the need for an error here because its possible to have a naming collision. In this case however the problem is because golang.org/x/net/context & context paths are actually the same package.

I'm not sure of a way to test two imports are the same but I made a less generic fix here: https://github.com/Rajczyk/mock

mbana commented 6 years ago

Please see this PR: https://github.com/golang/mock/pull/163.

It documents how you should be using mockgen when using grpc streams.

Summary: use the reflect invocation and that appear should disappear.

stevendanna commented 4 years ago

We've recently hit what I believe is a similar bug to this when trying to use mockgen on a protobuf generated go file that includes a streaming client. After looking at the code, I believe I know the root cause of the bug:

The inclusion of a streaming client forces mockgen to parse the grpc package for an interface definition that lives in the grpc package:

https://github.com/golang/mock/blob/master/mockgen/parse.go#L259

During this parsing, mockgen fails with an error:

2020/02/14 10:48:33 imported package collision: "backoff" imported twice

The supposed conflict is caused by the fact that the grpc package imports two different packages called backoff:

vendor/google.golang.org/grpc/clientconn.go
39: "google.golang.org/grpc/internal/backoff"

vendor/google.golang.org/grpc/backoff.go
19:// See internal/backoff package for the backoff implementation. This file is
27: "google.golang.org/grpc/backoff"

According to the Golang spec, imports are file-scoped, so these are legal imports. However, when parsing the package, mockgen merges all files in the package:

https://github.com/golang/mock/blob/master/mockgen/parse.go#L199

and then calls importsOfFile which does conflict detection as if they were all imported into the same file.

This issue and other related issues have had comments and labels indicating that this is considered a documentation bug and users should be using reflect mode rather than source mode. I can certainly change my local invocations to work around that for now, but given the analysis above I think this should be fixable. I'm happy to look into a solution here if y'all agree this is a bug.

Katiyman commented 4 years ago

My protoc version is 3.11.2 and grpc is 1.27.1 but i am facing same issue

codyoss commented 4 years ago

@stevendanna I would accept a PR that trys to make this better :smile_cat:

arjunsingri commented 4 years ago

What are the next steps here?

codyoss commented 4 years ago

@arjunsingri There has been a PR open to support this. In the meantime, as stated in this thread, you should be able to use reflect mode for now.

arjunsingri commented 4 years ago

@arjunsingri There has been a PR open to support this. In the meantime, as stated in this thread, you should be able to use reflect mode for now.

@codyoss

Can you please help me out and explain how to use the reflect mode? The doc says "list of symbols", and I wanted to understand what these symbols are.

Reflect mode generates mock interfaces by building a program that uses reflection to understand interfaces. It is enabled by passing two non-flag arguments: an import path, and a comma-separated list of symbols.

You can use "." to refer to the current path's package.

Example:

mockgen database/sql/driver Conn,Driver

# Convenient for `go:generate`.
mockgen . Conn,Driver
codyoss commented 4 years ago

Symbols == interface names