Add E-suffix container variants for TestMain usage #7
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds error-returning variants (E-suffix) of all container constructors to support usage in TestMain functions where *testing.T is not available. The E-suffix variants return (*Container, error) instead of using require.NoError internally, while the original functions now delegate to the E variants and call require.NoError(t, err).
Key changes:
- Added E-suffix variants for all 6 container types (PostgreSQL, MySQL, MongoDB, SSH, FTP, Localstack) - 9 functions total
- Implemented proper container cleanup on all error paths in E-suffix variants
- Updated README with TestMain usage example and E-suffix variants reference table
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| containers/ssh.go | Added NewSSHTestContainerE and NewSSHTestContainerWithUserE with proper error handling and container cleanup |
| containers/psql.go | Added NewPostgresTestContainerE and NewPostgresTestContainerWithDBE with error propagation |
| containers/mysql.go | Added NewMySQLTestContainerE and NewMySQLTestContainerWithDBE with error propagation |
| containers/mongo.go | Added NewMongoTestContainerE with environment variable handling and cleanup logic |
| containers/localstack.go | Added NewLocalstackTestContainerE with error handling |
| containers/ftp.go | Added NewFTPTestContainerE, removed test-specific logging code and helper methods |
| README.md | Added TestMain usage example and E-suffix variants reference table |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return []string{} | ||
| } | ||
| return strings.Split(cleanPath, "/") | ||
| } |
There was a problem hiding this comment.
The removed helper methods logContainerError and logContainerLogs may be useful for debugging container startup failures. While removing test-specific logging from the E-suffix variant is correct, consider whether these debugging utilities should be preserved elsewhere or if similar debugging capabilities should be available when using the E-suffix variants in TestMain contexts where troubleshooting container startup issues can be challenging.
| } | ||
|
|
||
| // connect function (Use default EPSV enabled) | ||
| // connect establishes an FTP connection and logs in |
There was a problem hiding this comment.
The comment was simplified but loses clarity. The original comment "connect function (Use default EPSV enabled)" provided context about the EPSV configuration. The new comment "connect establishes an FTP connection and logs in" doesn't mention this important detail about EPSV being enabled by default, which could be relevant for users troubleshooting connection issues.
| // connect establishes an FTP connection and logs in | |
| // connect establishes an FTP connection and logs in, using the default FTP client settings (EPSV enabled by default). |
containers/mongo.go
Outdated
| client, err := mongo.Connect(ctx, options.Client().ApplyURI(uri)) | ||
| require.NoError(t, err) | ||
| if err != nil { | ||
| _ = os.Setenv("MONGO_TEST", origURL) // restore original env value |
There was a problem hiding this comment.
The environment variable restoration may fail silently here. If origURL is an empty string (the variable was not set originally), this will set MONGO_TEST to an empty string rather than unsetting it. Consider using os.Unsetenv("MONGO_TEST") when origURL is empty to properly restore the original state.
| _ = os.Setenv("MONGO_TEST", origURL) // restore original env value | |
| if origURL != "" { | |
| _ = os.Setenv("MONGO_TEST", origURL) // restore original env value | |
| } else { | |
| _ = os.Unsetenv("MONGO_TEST") // restore original env state when it was unset | |
| } |
Add error-returning variants of all container constructors for use in TestMain where *testing.T is not available. E variants return (*Container, error) instead of using require.NoError internally. - Add NewPostgresTestContainerE and NewPostgresTestContainerWithDBE - Add NewMySQLTestContainerE and NewMySQLTestContainerWithDBE - Add NewMongoTestContainerE with env restoration on connect failure - Add NewSSHTestContainerE and NewSSHTestContainerWithUserE - Add NewFTPTestContainerE - Add NewLocalstackTestContainerE - Ensure container termination on all error paths - Update README with TestMain example and E-suffix variants table
umputun
force-pushed
the
add-e-suffix-container-variants
branch
from
4a62cc2 to
adc69a8
Compare
Summary
Add error-returning variants (E-suffix) of all container constructors for use in TestMain where
*testing.Tis not available.Changes
(*Container, error)instead of usingrequire.NoErrorinternallyrequire.NoError(t, err)MONGO_TESTenv value ifmongo.ConnectfailsE-suffix variants added
NewPostgresTestContainerNewPostgresTestContainerENewPostgresTestContainerWithDBNewPostgresTestContainerWithDBENewMySQLTestContainerNewMySQLTestContainerENewMySQLTestContainerWithDBNewMySQLTestContainerWithDBENewMongoTestContainerNewMongoTestContainerENewSSHTestContainerNewSSHTestContainerENewSSHTestContainerWithUserNewSSHTestContainerWithUserENewFTPTestContainerNewFTPTestContainerENewLocalstackTestContainerNewLocalstackTestContainerE