Skip to content

refactor: Update Docker container configuration for Cassandra in tests#118

Open
samof76 wants to merge 1 commit intomainfrom
fix-failing-tests
Open

refactor: Update Docker container configuration for Cassandra in tests#118
samof76 wants to merge 1 commit intomainfrom
fix-failing-tests

Conversation

@samof76
Copy link
Collaborator

@samof76 samof76 commented Dec 23, 2025

  • Added ExposedPorts and PortBindings for Cassandra in cql_test.go and lua_test.go.
  • Updated runDockerContainer function in utils.go to include port exposure and binding for PostgreSQL.
  • Ensured resource cleanup in runPostgresDB function only occurs if resource is not nil.

- Added ExposedPorts and PortBindings for Cassandra in cql_test.go and lua_test.go.
- Updated runDockerContainer function in utils.go to include port exposure and binding for PostgreSQL.
- Ensured resource cleanup in runPostgresDB function only occurs if resource is not nil.
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors Docker container configuration for test environments by standardizing port exposure and binding patterns across Cassandra and PostgreSQL containers, and adds defensive resource cleanup logic.

  • Standardizes port configuration by explicitly setting ExposedPorts and PortBindings for both Cassandra (port 9042) and PostgreSQL containers
  • Adds nil check before resource cleanup to prevent potential panic conditions
  • Disables automatic port publishing in favor of explicit port binding configuration

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
internal/psql/utils.go Added explicit PostgreSQL port configuration and nil check for resource cleanup
internal/lua/lua_test.go Configured explicit port exposure and binding for Cassandra container
internal/cql/cql_test.go Configured explicit port exposure and binding for Cassandra container

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +167 to +170
ExposedPorts: []string{postgresPort + "/tcp"},
PortBindings: map[dc.Port][]dc.PortBinding{
dc.Port(postgresPort + "/tcp"): {{HostIP: "", HostPort: ""}},
},
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PostgreSQL port configuration uses a variable 'postgresPort' while Cassandra configurations use hardcoded '9042/tcp'. This inconsistency makes the codebase harder to maintain. Consider either defining a constant for the Cassandra port or documenting why PostgreSQL uses a variable while Cassandra uses a hardcoded value.

Copilot uses AI. Check for mistakes.
Entrypoint: entrypointOverrides,
ExposedPorts: []string{postgresPort + "/tcp"},
PortBindings: map[dc.Port][]dc.PortBinding{
dc.Port(postgresPort + "/tcp"): {{HostIP: "", HostPort: ""}},
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty HostIP and HostPort values in PortBinding may lead to unexpected behavior. Consider documenting the intended behavior when these are empty strings, or explicitly set them to meaningful defaults if automatic assignment is not the desired outcome.

Suggested change
dc.Port(postgresPort + "/tcp"): {{HostIP: "", HostPort: ""}},
dc.Port(postgresPort + "/tcp"): {{HostIP: "0.0.0.0", HostPort: ""}},

Copilot uses AI. Check for mistakes.
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

Successfully merging this pull request may close these issues.

2 participants