fix(ci): fix E2E tests and docker network isolation#53
fix(ci): fix E2E tests and docker network isolation#53poyrazK merged 4 commits intofeature/elastic-ipfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes E2E test failures in CI by implementing proper Docker network isolation for parallel test runs. The changes enable multiple test runs to execute simultaneously without network conflicts by using dynamic network names based on GitHub run IDs.
Changes:
- Configured dynamic network isolation using
COMPOSE_PROJECT_NAMEwith GitHub run IDs - Propagated
DOCKER_DEFAULT_NETWORKconfiguration from environment to the API service - Standardized test utilities to use environment variables for Docker network configuration
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/helpers/helpers_test.go | Updated test assertions to use dynamic test base URL from testutil |
| tests/chaos_test.go | Changed from direct docker commands to docker compose commands for better container management |
| pkg/testutil/constants.go | Converted constants to variables with environment variable support for dynamic configuration |
| internal/core/services/instance_test.go | Added support for dynamic Docker network and test port configuration |
| internal/core/services/instance.go | Added dockerNetwork field and logic to use configured network instead of hardcoded value |
| internal/platform/config.go | Added DockerDefaultNetwork configuration field |
| internal/api/setup/dependencies.go | Passed DockerDefaultNetwork config to InstanceService initialization |
| docker-compose.yml | Removed static container names and added dynamic port/network configuration |
| .github/workflows/e2e.yml | Added COMPOSE_PROJECT_NAME and DOCKER_DEFAULT_NETWORK with run ID for isolation |
| .github/workflows/ci.yml | Added TEST_DOCKER_NETWORK environment variable and removed duplicate image tags |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var ( | ||
| // TestIPLocalhost is the loopback address used in tests. | ||
| TestIPLocalhost = "127.0.0.1" |
There was a problem hiding this comment.
Converting exported constants to variables reduces their immutability guarantees and could lead to unintended modifications during test execution. Consider keeping these as constants and only making the truly dynamic values (TestBaseURL, TestDatabaseURL, TestPort, TestProdPort, TestDockerNetwork) into variables, or use a function-based approach that returns computed values at call time.
| networks: | ||
| cloud-network: | ||
| name: cloud-network | ||
| cloud-network: # name: cloud-network |
There was a problem hiding this comment.
The inline comment # name: cloud-network appears to be leftover from removing the explicit name configuration. This comment should be removed or clarified to explain why the network name is not explicitly set.
| cloud-network: # name: cloud-network | |
| cloud-network: # Use implicit network name; explicit "name:" omitted intentionally |
| ghcr.io/${{ steps.repo_name.outputs.REPO_LC }}:staging | ||
| ghcr.io/${{ steps.repo_name.outputs.REPO_LC }}:${{ github.ref_name }} | ||
| ghcr.io/${{ steps.repo_name.outputs.REPO_LC }}:${{ github.sha }} |
There was a problem hiding this comment.
The removal of the ${{ github.ref_name }} tag is inconsistent with the stated purpose of this PR (fixing E2E tests and Docker network isolation). If this tag removal is intentional, it should be documented in the PR description or split into a separate commit/PR for clarity.
Description
This PR fixes the E2E test failures in CI by ensuring proper network isolation and configuration.
Changes
Verification