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

Return interfaces so that others using the lib can write unit tests. #164

Closed
urjitbhatia opened this issue Oct 1, 2015 · 4 comments
Closed

Comments

@urjitbhatia
Copy link

I am using this lib and trying to unit test my consumer and producer. I am new to go so I might just not be doing the right thing. However, I want to be able to mock out the Connection and/or the Channel and then write tests using those mocks. From my understanding, that would be really easy with a mocking framework (even otherwise) if the Dial method returned an interface which is implemented by Connection (connection.go) rather than the struct itself.

@streadway
Copy link
Owner

streadway commented Jan 6, 2017

The good thing about Go is that you can implement just the interfaces you need in your own code.

// You define your own interface of what you need satisfied from the amqp package
type Channel interface {
  QueueDeclare(...)
  Consume(...)
  Ack(...)
}

// Then your business logic depends on your interface with the dependency injected from `main` or elsewhere.
func businessLogic(channel Channel) {
 channel.QueueDeclare(...)
 deliveries, _ := channel.Consume(...)
 for delivery := range deliveries {
    ...
 }
}

// In tests you satisfy that interface with a mock
type mockChannel struct {
  queueDeclared bool
  deliveries chan amqp.Delivery
}

func (c *mockChannel) QueueDeclare(...) { c.queueDeclared = true }
func (c *mockChannel) Consume(...) { return c.deliveries }
func (c *mockChannel) Ack(...) { }

// Which means you can Unit test against a mock
func TestUnit(t *testing.T) {
  channel = &mockChannel{deliveries: make(chan amqp.Delivery)}
  go func() {
    channel.deliveries <- amqp.Delivery{Body: []byte("one")}
    channel.deliveries <- amqp.Delivery{Body: []byte("two")}
    channel.deliveries <- amqp.Delivery{Body: []byte("three")}
  }()

  businessLogic(channel)
  if !channel.queueDeclared {
    t.Fatal("didn't declare the queue")
  }
  // test businessLogic effects
}

// Or integration test against a connection
func TestIntegration(t *testing.T) {
  conn, err := amqp.DialConfig(...)
  channel, err := conn.Channel()
  businessLogic(channel)
  // test businessLogic effects
}

@mihaitodor
Copy link

mihaitodor commented Jan 9, 2017

@streadway @michaelklishin Here is an oversimplified (but, hopefully, still relevant) example of what I had to write in order to get around the fact that you don't return an interface from the Dial method:

package main

import "fmt"

// 3rd party library
type Channel struct {
}

type Connection struct {
	channel *Channel
}

func (c *Connection) Channel() *Channel {
	return c.channel
}

func Dial() *Connection {
	fmt.Println("Default Dial")

	return &Connection{}
}

// my code
type MyConnection struct {
	*Connection
}

type IChannel interface {
}

type IConnection interface {
	Channel() IChannel
}

func (c *MyConnection) Channel() IChannel {
	return c.Connection.Channel()
}

type App struct {
	connection IConnection
	dialer     func() IConnection
}

func MyDial() IConnection {
	fmt.Println("My Dial")

	conn := &MyConnection{}
	conn.Connection = Dial()

	return conn
}

func NewApp() *App {
	return &App{dialer: MyDial}
}

func (app *App) Connect() {
	app.connection = app.dialer()

	_ = app.connection.Channel()
}

// my test code
type MockConnection struct {
}

func (c *MockConnection) Channel() IChannel {
	return nil
}

func MockDial() IConnection {
	fmt.Println("Mock Dial")

	return &MockConnection{}
}

func NewMockApp() *App {
	return &App{dialer: MockDial}
}

func main() {
	// main code
	app := NewApp()
	app.Connect()

	//test code
	mockApp := NewMockApp()
	mockApp.Connect()
}

As you can see, I was forced to write a wrapper struct MyConnection with an embedded pointer to Connection as well as a wrapper for the Dial function, while all I wanted was to be able to inject MockDial into App.dialer inside a unit test. In my tests, I definitely want to be able to create a MockConnection struct which I want to return from MockDial instead of the real Connection.

Please note that Go does not support covariance, which is the reason why I couldn't simply assign Dial to App.dialer: cannot use Dial (type func() *Connection) as type func() IConnection in field value and I believe this is the issue which the OP was trying to point out.

However, if your Dial method would return an interface, I would be able to easily test my code without having to write superfluous wrappers:

package main

import "fmt"

// 3rd party library
type Channel struct {
}

type Connection struct {
	channel *Channel
}

func (c *Connection) Channel() IChannel {
	return c.channel
}

type IChannel interface {
}

type IConnection interface {
	Channel() IChannel
}

func Dial() IConnection {
	fmt.Println("Default Dial")

	return &Connection{}
}

// my code
type MyConnection struct {
	*Connection
}

type App struct {
	connection IConnection
	dialer     func() IConnection
}

func NewApp() *App {
	return &App{dialer: Dial}
}

func (app *App) Connect() {
	app.connection = app.dialer()

	_ = app.connection.Channel()
}

// my test code
type MockConnection struct {
}

func (c *MockConnection) Channel() IChannel {
	return nil
}

func MockDial() IConnection {
	fmt.Println("Mock Dial")

	return &MockConnection{}
}

func NewMockApp() *App {
	return &App{dialer: MockDial}
}

func main() {
	// main code
	app := NewApp()
	app.Connect()

	//test code
	mockApp := NewMockApp()
	mockApp.Connect()
}

Since this library has been around for some time, I am not sure what would be the implications of making such a change to its API layer (I think it shouldn't break exiting code, though), so I can understand if you don't want to change it.

@streadway
Copy link
Owner

Hi @mihaitodor,

Indeed, it would be a significant change to replace the returned structs with interfaces as it would break all callers that accept structs instead of interfaces. This kind of change would require a new package which is not planned.

When building apps, I make interfaces capture the smallest unit of logic possible. I almost always accept interfaces in functions and start with return concrete types until more than one implementation is needed. With this practice, I rarely need covariance of types.

I anticipated that most AMQP applications using this package would follow one of the messaging patterns described in Getting Started. I considered theses examples the units to test. In your application, this translates to "dialing a work queue", or "dialing a publishing sink".

The types in the amqp package model the AMQP wire protocol which makes them very concrete which is why they are structs. There is no other implementation that this package provides. If you need an open ended channel to mock, inject an open channel, or channel factory into your app instead of the dialer.

package main

// Just what you need from the amqp package
type Broker interface {
	QueueDeclare(/*...*/)
	Consume(/*...*/)
}

type App struct {
	NewBroker func(url string) (Broker, error)
}

func main() {
	app := App{
		NewBroker: func(url string) (Broker, error) {
			conn, err := amqp.Dial(url)
			if err != nil {
				return nil, err
			}
			return conn.Channel()
		},
	}

	mock := App{
		NewBroker: func() (Broker, error) { return &mockBroker{}, nil },
	}
}

If you move more to the application or messaging pattern boundary then it'd look more like:

type WorkQueue interface {
	io.Closer
	Get() (Job, error)
}

type App struct {
	NewWorkQueue func(brokerURL string) (WorkQueue, error)
}

I hope these recommendations help reduce the wiring code needed to make testable units in your code!

@mihaitodor
Copy link

@streadway Thanks for the detailed answer! It did clarify the intended usage.

Also, indeed, you're right, returning an interface will break existing code, because it requires a type assertion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants