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

Best approach for mocking container services #61

Open
akhayyat opened this issue Apr 27, 2024 · 6 comments
Open

Best approach for mocking container services #61

akhayyat opened this issue Apr 27, 2024 · 6 comments

Comments

@akhayyat
Copy link

Thank you @mikestefanello for this joyful template. It got me excited about building web applications in Go.

I have a custom service that I added to the container, that calls an external service (REST API). Any suggestions or recommendations for ways to test the application without making those REST API calls?

It would be great to say something about that in the README.

@mikestefanello
Copy link
Owner

You're very welcome. Thank you for the compliment. I'm glad to hear you're finding it useful.

Great question (and I agree, having something explicit about this in the README is a good idea).

What I'd recommend is similar to what was done for the database. Add this new service to the container as an interface and when initializing it in NewContainer() check if the environment is config.EnvTest, and if so, use a mock so you avoid making actual REST calls. Here is an example of that check: https://github.com/mikestefanello/pagoda/blob/main/pkg/services/container.go#L149. You just need to make sure you set the environment to test (ie, https://github.com/mikestefanello/pagoda/blob/main/pkg/services/services_test.go#L20-L25) prior to creating the container. For the DB, it connects to a test database rather than the primary database for tests.

To the best of my knowledge, there isn't an official way to determine if you're running within a test, so manually setting this environment is still required.

Let me know if that works for you.

@akhayyat
Copy link
Author

That worked!

However, I slightly dislike the following:

  1. Mixing test-related code with production code, to define and create the mock struct and behavior.
  2. Technically, the code path executed during tests is different from that executed in normal execution. I understand that this is ultimately unavoidable when using mocks, but having both code paths in the production code is different form the test clearly overriding/mocking specific functions as a part of the test setup code.

I was thinking about perhaps extending the DI approach to creating the service container itself, to enable swapping its own dependencies: the service clients. For example, allowing services.NewContainer() to optionally accept parameters that would override the corresponding services, such as services.NewContainer(db) to pass the test db connection (or a struct/interface that can create it). This way, test setup, such as services.TestMain (in services_test.go) can override whichever container dependencies to use test/mock ones.

Any thoughts on the current opaque container vs one that acknowledges its dependencies (or at least some of them) and allows them to be (optionally) passed as parameters?
Any advantages of the current approach over this?

@mikestefanello
Copy link
Owner

You bring up good points. Similar to what you suggested, what do you think about either of these?

  1. Create NewTestContainer() which is more explicit than checking the environment, and initializes everything in test/mock mode.
  2. In your tests (ie, in TestMain()) after you do c := NewContainer(), you can swap in mocks, like c.MyService = NewMockService(). I'm not a fan of this one because it's potentially duplicated, and you need to make sure you always remember to swap what you intend to.
  3. If the code you're testing using one or a very small amount of services on the container, you can just create a container in that test or test package with only the services you need, mocked however you want, etc.

@akhayyat
Copy link
Author

I tried option 1, and it seems to work. Here is what I tried:

  1. Remove all EnvTest related conditions from the services package and kept only production services. For example, initDatabase() in container.go now looks like:

    func (c *Container) getDatabaseAddr(dbName string) string {
        return fmt.Sprintf("postgresql://%s:%s@%s:%d/%s",
    	c.Config.Database.User,
    	c.Config.Database.Password,
    	c.Config.Database.Hostname,
    	c.Config.Database.Port,
    	dbName,
        )
    }
    
    // initDatabase initializes the database
    func (c *Container) initDatabase() {
        var err error
    
        c.Database, err = sql.Open("pgx", c.getDatabaseAddr(c.Config.Database.Database))
        if err != nil {
    	panic(fmt.Sprintf("failed to connect to database: %v", err))
        }
    }

    I had to move getAddr() outside of initDatabase() so I can use it in initTestDatabase() (see below).

  2. Implement NewTestContainer() and its test dependencies in services_test.go (in my case, I need a mock Auth service):

    func NewTestContainer() *Container {
        c := new(Container)
        c.initConfig()
        c.initValidator()
        c.initWeb()
        c.initTestCache()    // Test Cache
        c.initTestDatabase() // Test Database
        c.initORM()
        c.initTags()
        c.initChannels()
        c.initUsers()
        c.initMockAuth() // Mock Auth
        c.initTemplateRenderer()
        c.initMail()
        c.initTasks()
        return c
    }
    
    func (c *Container) initTestCache() {
        var err error
        if c.Cache, err = NewTestCacheClient(c.Config); err != nil {
    	panic(err)
        }
    }
    
    func (c *Container) initTestDatabase() {
        var err error
    
        // Connect to the test database server
        c.Database, err = sql.Open("pgx", c.getDatabaseAddr("postgres"))
        if err != nil {
    	panic(fmt.Sprintf("failed to connect to database: %v", err))
        }
    
        // Drop the test database, ignoring errors in case it doesn't yet exist
        _, _ = c.Database.Exec("DROP DATABASE " + c.Config.Database.TestDatabase)
    
        // Create the test database
        if _, err := c.Database.Exec("CREATE DATABASE " + c.Config.Database.TestDatabase); err != nil {
    	panic(fmt.Sprintf("failed to create test database: %v", err))
        }
    
        c.Database, err = sql.Open("pgx", c.getDatabaseAddr(c.Config.Database.TestDatabase))
        if err != nil {
    	panic(fmt.Sprintf("failed to connect to database: %v", err))
        }
    }
    
    func (c *Container) initMockAuth() {
        c.Auth = newMockAuthClient(c.Users)
    }
  3. Implement NewTest[Service]Client() (or NewMock[Service]Client()) in the service's respective [service]_test.go. For example, in cache_test.go:

    func NewTestCacheClient(cfg *config.Config) (*CacheClient, error) {
        db := cfg.Cache.TestDatabase
    
        // Connect to the cache
        c := &CacheClient{}
        c.Client = redis.NewClient(&redis.Options{
    	Addr:     fmt.Sprintf("%s:%d", cfg.Cache.Hostname, cfg.Cache.Port),
    	Password: cfg.Cache.Password,
    	DB:       db,
        })
        if _, err := c.Client.Ping(context.Background()).Result(); err != nil {
    	return c, err
        }
    
        // Flush the database in the test environment
        if err := c.Client.FlushDB(context.Background()).Err(); err != nil {
    	return c, err
        }
    
        cacheStore := redisstore.NewRedis(c.Client)
        c.cache = cache.New[any](cacheStore)
        return c, nil
    }

Comments:

  1. I like that all test code is exclusively in *_test.go files, and none of it leaks into production code.
  2. I'm not sure about mixing code for test cases with code for test setup, especially in [service]_test.go files as shown in cache_test.go. If there is a way to separate them, that'd be great.
  3. I initially tried to keep NewTestContainer() and related test setup code (e.g. initTestDatabase()) in the tests package (pkg/tests), but ran into dependency cycles.
  4. Any comments about the locations of each piece of the test code? Any suggested improvements?
  5. I was expecting DI to offer a cleaner way of injecting mock/test implementations. I have to say, I'm not very experienced with DI, but this is one of the most cited advantages of DI, but I can't see it.

About option 2: I share your dislike to option 2, since it unnecessarily creates the non-test clients then overwrites them.

About option 3: I think option 3 is a variation of option 1, in which NewTestContainer() creates a subset of the services. The different containers for different tests would require somewhat redundant code. Instead of duplicating the NewTestContainer() code with different created services, I'd rather have that code once and reuse it.

@akhayyat
Copy link
Author

Keeping NewTestContainer() in services_test.go is a no-go :/
It cannot be imported in other packages tests, e.g. routes/routes_test.go.

@mikestefanello
Copy link
Owner

Sorry for the delay. I was unavailable the last week.

As you pointed out, you can't share code in test files across packages. I've ran in to that many times and it's a bit annoying, but understandable.

You'll face the same issues no matter what your DI approach is. The container is really just a wrapper around multiple dependencies and makes it a bit easier to pass things around (.. I think). I personally don't worry about having mock clients reside outside of test code. If I write a package with a given service (ie, NewCoolClient()), I often include a mock constructor as well (ie, NewMockCoolClient()) especially if other packages are using that as a dependency.

Without seeing the rest of your project, it's hard to evaluate your options and give any more guidance. It sounds like you're on the right path though.

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

2 participants