Refactor tests for improved clarity and maintainability#44
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the integration-test bootstrap pattern to use a functional setup callback that receives a pg.PoolConn and returns an optional cleanup function, updating test suites and documentation to match the new approach.
Changes:
- Updated
pkg/test.Mainto acceptsetup func(pg.PoolConn) (func(), error)and adjusted internal setup/cleanup handling. - Updated package
TestMainimplementations to initialize their package-leveltest.Connvia the new setup callback. - Updated
pkg/testdocumentation (README + package docs) to reflect the new pattern andBeginreturn type change.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pool_test.go | Updates root integration tests to initialize the global test connection via the new setup callback. |
| pkg/test/main.go | Refactors Main signature and setup plumbing; changes Conn.Begin to return pg.PoolConn. |
| pkg/test/doc.go | Updates package-level usage example to the new Main pattern. |
| pkg/test/README.md | Updates README usage snippet to the new Main setup callback pattern. |
| pkg/queue/manager_test.go | Updates queue tests’ TestMain to the new setup callback pattern. |
| pkg/manager/database_test.go | Updates manager tests’ TestMain to the new setup callback pattern. |
| pkg/broadcaster/broadcaster_test.go | Updates broadcaster tests’ TestMain to the new setup callback pattern. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+79
to
80
| cleanup_, err := setup(types.Ptr(Conn{pool, nil})) | ||
| if err != nil { |
Comment on lines
+13
to
+19
| // // Start up a container and return the connection | ||
| // func TestMain(m *testing.M) { | ||
| // test.Main(m, func(pool pg.PoolConn) (func(), error) { | ||
| // conn = test.Conn{PoolConn: pool} | ||
| // return nil, nil | ||
| // }) | ||
| // } |
Comment on lines
23
to
+27
| func TestMain(m *testing.M) { | ||
| pgtest.Main(m, &conn, nil) | ||
| pgtest.Main(m, func(pool pg.PoolConn) (func(), error) { | ||
| conn = pgtest.Conn{PoolConn: pool} | ||
| return nil, nil | ||
| }) |
Comment on lines
77
to
80
| cleanup := func() {} | ||
| if setup != nil { | ||
| cleanup_, err := setup(conn) | ||
| cleanup_, err := setup(types.Ptr(Conn{pool, nil})) | ||
| if err != nil { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request refactors the test setup pattern across several packages to use a functional approach for initializing database connections in tests. The main change is replacing the previous pattern of passing a pointer to a connection struct into the test helper with a setup function that receives a
pg.PoolConnand returns an optional cleanup function. This improves flexibility and consistency in test initialization.Test setup refactoring:
Updated the
test.Mainandpgtest.Mainfunctions (and their usage in test files) to accept a setup function that receives apg.PoolConnand returns a cleanup function, instead of passing a pointer to a connection struct and a setup function. This change affectspkg/test/main.go,pkg/broadcaster/broadcaster_test.go,pkg/manager/database_test.go,pkg/queue/manager_test.go, andpool_test.go. [1] [2] [3] [4] [5] [6]Updated documentation and usage examples in
pkg/test/README.mdandpkg/test/doc.goto reflect the new setup pattern for test initialization. [1] [2]Code improvements and minor changes:
Changed the
Beginmethod onConnto return apg.PoolConninterface instead of a pointer toConn, improving abstraction.Added a missing import for
typesinpkg/test/main.goto support the use oftypes.Ptr.Internally, the main test helper now constructs and passes the connection struct via the setup function, rather than setting it via pointer assignment.
These changes make the test setup more modular and easier to extend for future needs.