fix: Docker E2E tests skip in CI unless explicitly opted in#253
Merged
anandgupta42 merged 1 commit intomainfrom Mar 18, 2026
Merged
fix: Docker E2E tests skip in CI unless explicitly opted in#253anandgupta42 merged 1 commit intomainfrom
anandgupta42 merged 1 commit intomainfrom
Conversation
GitHub Actions runners have Docker pre-installed, so isDockerAvailable() returns true. But the TypeScript CI job has no test databases running, causing 3 tests to hang for 60-90s then fail. Fix: require DRIVER_E2E_DOCKER=1 env var to run Docker-based tests. The driver-e2e CI job sets TEST_*_HOST vars (CI services), which still work. Local dev can set DRIVER_E2E_DOCKER=1 to test with Docker. This eliminates 3 of the 14 CI failures on main. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Claude Code ReviewThis repository is configured for manual code reviews. Comment |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
|
|
||
| function isDockerAvailable(): boolean { | ||
| if (process.env.TEST_PG_HOST) return true // CI services replace Docker | ||
| if (!process.env.DRIVER_E2E_DOCKER) return false // Skip unless opted in |
There was a problem hiding this comment.
Bug: The check for DRIVER_E2E_DOCKER is a loose truthy check, not a strict === "1" check, causing it to run tests even when a user explicitly opts out with a value like "0".
Severity: MEDIUM
Suggested Fix
Update the condition in drivers-e2e.test.ts to match the strict check used in other test files. Change the check from if (!process.env.DRIVER_E2E_DOCKER) to if (process.env.DRIVER_E2E_DOCKER !== "1") to ensure that tests only run when the variable is explicitly set to "1".
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: packages/opencode/test/altimate/drivers-e2e.test.ts#L37
Potential issue: The check for the `DRIVER_E2E_DOCKER` environment variable is
inconsistent between two test files. In `drivers-e2e.test.ts`, the condition
`!process.env.DRIVER_E2E_DOCKER` is used to skip Docker tests. This is a truthy check,
meaning if a developer sets `DRIVER_E2E_DOCKER=0` or `DRIVER_E2E_DOCKER=false` to
explicitly opt-out, the check `!"0"` evaluates to `false`, and the tests will
incorrectly proceed to run with Docker. This contradicts the behavior in
`drivers-docker-e2e.test.ts`, which uses a strict `=== "1"` check. This inconsistency
can cause unexpected test execution and failures in local development environments.
Did we get this right? 👍 / 👎 to inform future reviews.
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
1 participant
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.
3 tests hang for 60-90s in CI because Docker exists but no test DBs are running. Now requires DRIVER_E2E_DOCKER=1 to run locally.