go-github-mock icon indicating copy to clipboard operation
go-github-mock copied to clipboard

Suggestions and findings

Open Marakai opened this issue 1 year ago • 5 comments

This is very useful and saved me quite some time, when I otherwise would have had to do this "by hand" when integrating Github tests into our testify based unit tests.

Is this still being actively developed and supported?

I've had a look at the source and noticed something that are causing issues:

WithRequestMatch is a simple FIFO queue, which makes it hard to use the package in testify suites: without a clear, deterministic order of test invocation, you don't know which response will be encountered and whether it "matches" the test being run.

Also, it would be nice if there were a way to retrieve mocks based on the parameter passed in, for example users/{user}/repos: I may want a different reponse for "joe" versus "thisuserdoesntexist". At first glance it wasn't obvious how to implement that without ripping the entire queue approach apart.

As a result you have to keep the mock results within a test, which really bloats the test when you also already have test tables shoved in there.

Marakai avatar Sep 14 '22 08:09 Marakai

Hi @Marakai ,

Is this still being actively developed and supported?

Yep!

WithRequestMatch is a simple FIFO queue, which makes it hard to use the package in testify suites: without a clear, deterministic order of test invocation, you don't know which response will be encountered and whether it "matches" the test being run.

I actually thought of this when I was first drafting this code. The overall mock server wasn't designed to be used in concurrent access really. You might be able to go around this by having specific mock variable for each test instead of keeping a wider reference to the mock on the wider "test suite". (I guess that's the terminology testify uses)

If you don't want to go down this path, there's still the option of using the lower level WithRequestMatchHandler which will give you almost-full access to setup the server as you see fit. You will probably need to implement something else that implements the http.Handler to get around of not using the FIFOResponseHandler.

Also, it would be nice if there were a way to retrieve mocks based on the parameter passed in, for example users/{user}/repos: I may want a different reponse for "joe" versus "thisuserdoesntexist". At first glance it wasn't obvious how to implement that without ripping the entire queue approach apart.

I reckon this is currently possible. It just requires some extra steps from you.

You could define another EndpointPattern with the specific endpoint you want and Gorilla internally should match it correctly and return your mocked response as expected.

As a result you have to keep the mock results within a test, which really bloats the test when you also already have test tables shoved in there.

You can still have a somewhat global reference to a bunch of MockBackendOptions or even lower level handlers and just rearrange them in the order you need for each specific tests instead of recreating the whole setup everytime. With this you can save up some space and hopefullyl make it less bloated.

migueleliasweb avatar Sep 14 '22 11:09 migueleliasweb

I literally arrived at pretty much the same ideas while in trying to go sleep last night. My brain works funny. The mock variables approach should still be fairly clean and compact.

I haven't yet looked into defining my own EndpointPattern but was wondering if that's possible, so good to hear that you indicate it should be. You wouldn't have a snippet for the above requirement, perchance?

Thanks for the swift reply!

Marakai avatar Sep 14 '22 23:09 Marakai

I now use a quick & dirty map, with the Testify suite doing most of the heavy lifting in making sure this is called and set. As you suggested I use separate mocks for each test:

func (suite *GithubTestSuite) BeforeTest(suiteName, testName string) {
	m := map[string]*http.Client{
		"TestGithubFacade_CreatePullRequestTopLevelComment": mockHttpClientCreatePullRequestTopLevelComment(),
	}
	suite.httpClient = m[testName]
	suite.ghClient = github.NewClient(suite.httpClient)
}

Marakai avatar Sep 15 '22 02:09 Marakai

I haven't yet looked into defining my own EndpointPattern but was wondering if that's possible, so good to hear that you indicate it should be. You wouldn't have a snippet for the above requirement, perchance?

The EndpointPattern is just a small struct that contains the basic information to set the Gorilla (the server mux) handler.

As long as you have a URI and method that matches the request you're doing, you should be able to match very specific requests.

See: https://github.com/migueleliasweb/go-github-mock/blob/master/src/mock/server_options.go#L34

migueleliasweb avatar Sep 15 '22 04:09 migueleliasweb

Oh, that seems about as simple as it gets. :) I'll have a go!

Marakai avatar Sep 16 '22 05:09 Marakai

HI @Marakai , I will close this issue for now.

Feel free to reach out if you encounter some other issue.

migueleliasweb avatar Oct 01 '22 11:10 migueleliasweb

This is now working beautifully and is integrated into our prod CI pipelines :)

Marakai avatar Nov 01 '22 04:11 Marakai