Add Client interface/mock to allow Mocked testing

[artheus-sbab]

Right now, the Github client is very hard to use for unit tests for projects depending on this.

It would be a great idea to create an Interface for the client. Much like the K8S client mocked client, in the Go client project.

[gmlewis]

Actually, one of the most beautiful things about the Go programming language is that you can create interfaces in your own code without any external dependency dictating what your interface should be.

Please take a look at the following issues to give you an idea on how to proceed:

• Interfacing the github Client #1596 (comment)
• Mock Create repo/issue calls #1181 (comment)
• Just a simple "Thank you" #860

If you are having a specific problem mocking any particular component, please share your problems here and one of our amazing volunteers should be able to give you an example of how to mock the interface in your own code.

[artheus-sbab]

@gmlewis Well, to be quite honest. I do not really agree with that when the interface you need is to a Dependency.
One never knows when or how the dependency will change, therefore I see it as necessary to provide an interface whenever writing a client library to any kind of API.

[gmlewis]

The problem with providing an interface to all methods in a service is that you then need to provide implementations of all the methods in the interface when you mock it, which gets very old very fast. Whereas if you write your own interface, you can mock only the individual methods that you are actually using.

Do you have a specific problem in writing your unit tests, or were you just wanting to point out the philosophical differences between this repo and K8S ?

By the way, it is simple enough to write a Go script that would pump out an interface for every method in this repo, but again I think this is a genuinely bad idea.

[gmlewis]

One never knows when or how the dependency will change, therefore I see it as necessary to provide an interface whenever writing a client library to any kind of API.

Maybe I'm missing the point. Let's say we added an interface to every single method in this repo (which again, should be trivial to do)... how would this actually be used?

In other words, how is having an interface to every method any better than just letting the compiler tell you whether you are calling each method correctly or not?

[migueleliasweb]

The problem with providing an interface to all methods in a service is that you then need to provide implementations of all the methods in the interface when you mock it, which gets very old very fast. Whereas if you write your own interface, you can mock only the individual methods that you are actually using.

Do you have a specific problem in writing your unit tests, or were you just wanting to point out the philosophical differences between this repo and K8S ?

By the way, it is simple enough to write a Go script that would pump out an interface for every method in this repo, but again I think this is a genuinely bad idea.

Hi @gmlewis , you might not be fully aware that you can embed interfaces into structs in a way to automatically adhere to them. This allows you to just implement the methods you are actually calling for each scenario.

I totally agree with @artheus-sbab . Just like many other important libs for services like GCP, AWS, K8s provide interfaces for all of their services, this one should provide one too.

Here an example of what I'm saying:

package main

type Iface interface {
	Foo() string
	Bar() string
}

type FakeIFace struct {
	Iface // this allows FakeIface to automatically adhere to the interface

	// the caveat here is that if we actually call Foo() or Bar() this will cause a panic
	// but in unittests where we know which calls are being made, this is a non-issue
	// as we strategically implement only the required methods for that scenario
}

func (f *FakeIFace) Bar() string {
	return "Fake Bar()"
}

func TestIFace(f Iface) string {
	// in this specific usage of the interface
	// we know we only call one of the methods
	// and since we've embeded the interface
	// we can only override one of them
	return f.Bar()
}

func main() {
	// lets pretent this is a unittest

	fakeIFace := FakeIFace{}

	TestIFace(&fakeIFace)

}

Or if you prefer, https://play.golang.org/p/fU94md71VlS

[gmlewis]

You are absolutely right, @migueleliasweb - I was not aware of the embedding aspect of interfaces... I apologize... and thank you for enlightening me.

This should be a simple matter to create them when I have a moment.

If you wouldn't mind, maybe you can test out my PR once I write it and see if I got it right.

[migueleliasweb]

Absolutely, @gmlewis ! I also wouldn't mind creating a draft myself of what I was thinking and we could go from there.

👍👍👍

[gmlewis]

Oh, OK, sure... Go ahead @migueleliasweb . It's yours.

[gmlewis]

Note, @migueleliasweb , that of most importance is that it solves the problem and is easy to maintain which means it needs to update automatically with go generate ./....

[migueleliasweb]

It's night time for me rn. I'll give a crack on this tomorrow after work. I will keep you posted!

[gmlewis]

@migueleliasweb - I've created my implementation in #1832. Please take a look and see if this is OK with you.

[willnorris]

I totally agree with @artheus-sbab . Just like many other important libs for services like GCP, AWS, K8s provide interfaces for all of their services, this one should provide one too.

@migueleliasweb would you mind providing some examples of this, just to get a better idea of how others have handle this?

[migueleliasweb]

I totally agree with @artheus-sbab . Just like many other important libs for services like GCP, AWS, K8s provide interfaces for all of their services, this one should provide one too.

@migueleliasweb would you mind providing some examples of this, just to get a better idea of how others have handle this?

Sure can, @willnorris !

These AWS ones are fresh in my mind as I've been using them quite a bit recently:

• https://docs.aws.amazon.com/sdk-for-go/api/service/sqs/sqsiface/
• https://docs.aws.amazon.com/sdk-for-go/api/service/dynamodb/dynamodbiface/

[rspier]

	// the caveat here is that if we actually call Foo() or Bar() this will cause a panic
	// but in unittests where we know which calls are being made, this is a non-issue
	// as we strategically implement only the required methods for that scenario

This bit is a little worrying. In tests it's not a problem, but are there cases where this nil might leak into production code?

https://eli.thegreenplace.net/2020/embedding-in-go-part-3-interfaces-in-structs/ goes into a little more detail about the general approach.

[migueleliasweb]

This bit is a little worrying. In tests it's not a problem, but are there cases where this nil might leak into production code?

https://eli.thegreenplace.net/2020/embedding-in-go-part-3-interfaces-in-structs/ goes into a little more detail about the general approach.

I reckon the chances of the interface embedding "leaking" into production code is very very low as no one would have to deal with the interface directly in the first place. As of for tests, as you already mentioned in another comment, the benefits are that the final user can isolate their code to that of the github client by creating their own implementation/mocks of the required methods.

[sagar23sj]

The problem with providing an interface to all methods in a service is that you then need to provide implementations of all the methods in the interface when you mock it, which gets very old very fast. Whereas if you write your own interface, you can mock only the individual methods that you are actually using.
Do you have a specific problem in writing your unit tests, or were you just wanting to point out the philosophical differences between this repo and K8S ?
By the way, it is simple enough to write a Go script that would pump out an interface for every method in this repo, but again I think this is a genuinely bad idea.

Hi @gmlewis , you might not be fully aware that you can embed interfaces into structs in a way to automatically adhere to them. This allows you to just implement the methods you are actually calling for each scenario.

I totally agree with @artheus-sbab . Just like many other important libs for services like GCP, AWS, K8s provide interfaces for all of their services, this one should provide one too.

Here an example of what I'm saying:

package main

type Iface interface {
	Foo() string
	Bar() string
}

type FakeIFace struct {
	Iface // this allows FakeIface to automatically adhere to the interface

	// the caveat here is that if we actually call Foo() or Bar() this will cause a panic
	// but in unittests where we know which calls are being made, this is a non-issue
	// as we strategically implement only the required methods for that scenario
}

func (f *FakeIFace) Bar() string {
	return "Fake Bar()"
}

func TestIFace(f Iface) string {
	// in this specific usage of the interface
	// we know we only call one of the methods
	// and since we've embeded the interface
	// we can only override one of them
	return f.Bar()
}

func main() {
	// lets pretent this is a unittest

	fakeIFace := FakeIFace{}

	TestIFace(&fakeIFace)

}

Or if you prefer, https://play.golang.org/p/fU94md71VlS

Hi @migueleliasweb, could you please link the part of the code (client interface) from K8s or AWS, that you talked about in the above conversation. I am new to golang and trying to understand the above conversation about interfaces and testing which btw is very enlightening. Maybe looking at the code can help me understand better.
Thanks 😃

[migueleliasweb]

These AWS ones are fresh in my mind as I've been using them quite a bit recently:

• https://docs.aws.amazon.com/sdk-for-go/api/service/sqs/sqsiface/
• https://docs.aws.amazon.com/sdk-for-go/api/service/dynamodb/dynamodbiface/

Hi @sagar23sj , have a look at these ones.

[sagar23sj]

These AWS ones are fresh in my mind as I've been using them quite a bit recently:

• https://docs.aws.amazon.com/sdk-for-go/api/service/sqs/sqsiface/
• https://docs.aws.amazon.com/sdk-for-go/api/service/dynamodb/dynamodbiface/

Hi @sagar23sj , have a look at these ones.

Thanks @migueleliasweb , somehow I missed these from above 😅

[gmlewis]

@migueleliasweb created a new repo to help with mock testing: https://github.com/migueleliasweb/go-github-mock
Thank you, @migueleliasweb !
Closing issue.