Added join and expectations functions to the sqlmock interface.

[bernielomax]

I have added two new functions to the sqlmock interface:

• Expectations() []expectation
• Join(sqlmock Sqlmock)

This means you can now predefine mocks and reuse them as per the next example.

func MockFindTweeter() (*sql.DB, sqlmock.Sqlmock, error) {

	db, mock, err := sqlmock.New()
	if err != nil {
		return nil, nil, err
	}

	mock.ExpectPrepare("FIND (.+) FROM tweeters").
		ExpectQuery().
		WithArgs(tweeterID).
		WillReturnRows(TweeterRows())

	return db, mock, nil
}

func MockFindTweets() (*sql.DB, sqlmock.Sqlmock, error) {

	db, mock, err := sqlmock.New()
	if err != nil {
		return nil, nil, err
	}

	mock.ExpectPrepare("FIND (.+) FROM tweets").
		ExpectQuery().
		WithArgs(tweeterID).
		WillReturnRows(TweetRows())

	return db, mock, nil
}

db, mock, err := database.MockFindTweeter()
if err != nil {
	t.Fatal(err)
}

_, mockFindTweets, err := database.MockFindTweets()
if err != nil {
	t.Fatal(err)
}

mock.Join(mockFindTweets)

I would have preferred to have just been able to access sqlmock.expectations directly. Im not sure what the reasoning is for not exporting expectations or the expectations interface.

[codecov-io]

Codecov Report

Merging #97 into master will decrease coverage by 0.55%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master      #97      +/-   ##
==========================================
- Coverage   89.66%   89.11%   -0.56%     
==========================================
  Files          12       12              
  Lines         648      652       +4     
==========================================
  Hits          581      581              
- Misses         47       51       +4     
  Partials       20       20

Impacted Files	Coverage Δ
sqlmock.go	88.88% <0%> (-1.26%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 08090c7...2ac4e59. Read the comment docs.

[l3pp4rd]

Hi, none of this will be accepted. Adding any method to public library interface is expensive and have to be well thought before doing it. In this case, if you add sqlmock.Join(...sqlmock.Sqlmock). We would just create more ways to do the same thing, there are many ways to achieve the same result, best of them is to have a method like MockFindTweets(mock Sqlmock), which would mock something with a Sqlmock interface as a reference and expect what you need in each of those methods. That is a simple implementation and does not require any extensions on this library interface.

If you can make the same thing using the library in different ways, means that it is confusing. Every exported property or new method, which just adds some quicker way to do something already possible, makes it more confusing and harder to understand for library users.

By the way, when you contribute to golang project, use go fmt. And make sure you spend enough time to understand what your change adds to the library, because now it seems like just a convenience patch for your own use. Also you made the same doc block for both exported methods.

If you have read the contribution guide in README.md, there was a strong recommendation to open an issue first concerning interface changes, that way we would have wasted less time on this.

[bernielomax]

Thank you for your kind words of encouragement. My heart is full. 90% of people I speak with on gopers.slack.com tell me to stay well away from this library. Their comments are that its such a massive waste of coding overhead for very little gain. I do not want to have to constantly redefine expectations that do the same thing. They should be just exported in the first place. Let the people decided how they want to manage them and what is expensive and what is not. It must have taken you seconds to look at this PR. Thank you for MY time.

[l3pp4rd]

Well, it is sad to hear that many people suggest you not to use the library, maybe they do not understand what is unit test or integration test most likely. But my point was that MockFindTweets(mock Sqlmock) would work the same way as your proposed interface extension. I did not want to sound rude, probably same while you were making this pull request

[bernielomax]

Thanks @l3pp4rd but your response to my PR did lack courtesy. Granted I should have read the contributing section of the README prior to raising a PR but I did not deserve your response. I was only trying to help. I knew my code wasn't great but thats only because of the amounts of constraints in the existing code. I just thought it would be a starting point that could be discussed and refactored where need be.

Your suggestion of MockFindTweets(mock Sqlmock) only works for the example in the PR which I tried to keep fairly simple for clarity. For other scenarios using that method still means a lot of code replication and scaffolding to be able to achieve this. Id be happy to discuss what I mean in more detail if it is worthwhile.

I think the main problem people have is the amount of code duplication required to use the library. The amount of man hours required to maintain the code given the limitations set by not opening it up a little more just makes the library not feasible. I think people do understand what the library can do and why you would use it. However like me, people just cant justify using it.

What is expensive to you in code performance, to me is nothing in comparison to the expense of human resource required to maintain it. Just having to use New() to create a Sqlmock is a waste of code to me. That gets replicated a thousand times in order to cover every mock case. If I were to use the MockFindTweets(mock Sqlmock) method you suggested, *sql.DB and error would be continually thrown away just so you can pass the mock around.

As a developer I should be able to decide if want to declare/create a new mock db or sqlmock, or both. Same goes for expectations. Yes it does make the library a little harder to learn but it also makes them way more powerful. How would "net/http" be if 90% of it was hidden away from exports because it might be confusing to the developer.

[l3pp4rd]

I'm always open to discuss these constraints, you think are getting in your way by using this library. I understand, that creating sqlmock.New in every test, is cumbersome, but same for checking error in go. Though, you have choices to abstract that away, by making a mock builder in your tests, for example:

db, asserter, err := dbMock.New().FindTweets(id, rows).FindTweeter(id).build()

Where for example asserter in this case is a func, which checks if all expectations were met.

asserter := func(t *testing.T) {
        if err := mock.ExpectationsWereMet(); err != nil {
		t.Errorf("there were unfulfilled expections: %v", err)
	}
}

Also the testing.Helper can be used to add additional helper methods. I do not want to add similar logic or extensions to sqlmock library itself, because there are many ways to abstract such logic and one in library might not be the best option for user. It would be same as saying, "here is the way you should do it"

[bernielomax]

I think db, asserter, err := dbMock.New().FindTweets(id, rows).FindTweeter(id).build() method could work quite well for defining the mock expect execution order. However Im still not sure it will cut down on much code. What about testing for begin, prepare, commit failures, etc? Would I have to define variants of FindTweets such as FindTweetsShouldFailOnPrepare , etc. Or would these be options that you have to pass into FindTweets and let the logic determine what to expect before passing on the sqlmock.

Just to clarify in regards to your example above. You are suggesting that I create a struct similar to the one below:

type Builder struct {
	DB    *sql.DB
	Mock  sqlmock.Sqlmock
	Error error
}

Pass that through FindTweets, FindTweeter, etc. The Build function will return the Builder structs db, assertion and err fields?

[l3pp4rd]

You may extend Sqlmock interface with your builder and bind these extra methods.

type Builder struct {
        sqlmock.Sqlmock

	DB    *sql.DB
	Error error
}

Method like: FindTweets on that builder could return ExpectedQuery, so you could modify it with error and etc. But for example produce the correct sql query matcher for select statement.

My point is, that you may go far with abstraction, and I doubt that sqlmock library can make something more sophisticated compared to what you can in your custom builder implementation.