Conversation
…chore/componentise-feature-override-row # Conflicts: # frontend/web/components/feature-override/FeatureOverrideRow.tsx
…ponentise-feature-filters
…ponentise-feature-filters
…ponentise-feature-filters
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| const CHANNEL_ID = 'C0102JZRG3G'; // infra_tests channel ID | ||
| const failedJsonPath = path.join(__dirname, 'test-results', 'failed.json'); | ||
| const failedData = JSON.parse(fs.readFileSync(failedJsonPath, 'utf-8')); | ||
| const failedCount = failedData.failedTests?.length || 0; |
There was a problem hiding this comment.
File read crashes before existence check is reached
High Severity
fs.readFileSync(failedJsonPath, 'utf-8') on line 8 executes unconditionally at module load, before the fs.existsSync guard on line 82 is ever reached. If failed.json doesn't exist, the script crashes with an unhandled ENOENT error. Since this script only runs on CI test failures (if: failure()), the file may well be absent — for example, when global setup fails before any test executes. The existence check and early process.exit(0) are dead code in the missing-file scenario. Moving the file read after the existence check (or into main()) would fix this.
Additional Locations (1)
| API_IMAGE: ${{ inputs.api-image }} | ||
| E2E_IMAGE: ${{ inputs.e2e-image }} | ||
| E2E_CONCURRENCY: ${{ inputs.concurrency }} | ||
| E2E_RETRIES: 2 |
There was a problem hiding this comment.
E2E_RETRIES not forwarded to Docker container in CI
Medium Severity
E2E_RETRIES: 2 is set in the CI workflow step env but never reaches the Docker container. The Makefile explicitly forwards E2E_CONCURRENCY to the container via cross-env in the sh -c command, but E2E_RETRIES is not included. The docker-compose-e2e-tests.yml frontend service environment section also doesn't list it. So run-with-retry.ts inside the container always uses the default value of 1, making the CI setting ineffective.
Additional Locations (1)
Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| const CHANNEL_ID = 'C0102JZRG3G'; // infra_tests channel ID | ||
| const failedJsonPath = path.join(__dirname, 'test-results', 'failed.json'); | ||
| const failedData = JSON.parse(fs.readFileSync(failedJsonPath, 'utf-8')); | ||
| const failedCount = failedData.failedTests?.length || 0; |
There was a problem hiding this comment.
File read crashes before existence check executes
Medium Severity
The top-level fs.readFileSync(failedJsonPath, 'utf-8') on line 8 executes at module load time, before the fs.existsSync(failedJsonPath) guard on line 82. If failed.json doesn't exist (e.g., global setup failure prevents any tests from running), the script crashes with an unhandled error instead of gracefully exiting. The existence check needs to be moved before the file read.
Additional Locations (1)
| || (docker compose logs flagsmith-api; exit 1) | ||
| @echo "Running E2E tests..." | ||
| @docker compose run --name e2e-test-run frontend \ | ||
| sh -c 'npx cross-env E2E_CONCURRENCY=${E2E_CONCURRENCY} npm run test -- $(opts)' \ |
There was a problem hiding this comment.
E2E_RETRIES not forwarded to Docker container
Medium Severity
The Makefile test target passes E2E_CONCURRENCY via cross-env to the Docker container but omits E2E_RETRIES. The CI workflow sets E2E_RETRIES: 2 as a step-level env var, but since the Makefile doesn't include it in the cross-env command and it's not in docker-compose-e2e-tests.yml, the test runner inside the container defaults to 1 retry instead of the intended 2.
Additional Locations (1)
|
Great work on the Playwright migration @kyle-ssg ! The DX improvements (trace viewer, DOM snapshots, interactive UI mode) are huge. I tested extensively using both Test ResultsUsing Using Using Observations1. Enterprise tests fail against OSS API Two tests consistently fail locally:
Root cause: 2. Concurrency mismatch
3. Found instances in CI handles OSS/Enterprise correctlyLooking at
Can't build private-cloud locallyTried changing to Questions
None of these are blockers. The PR is really good and ready to merge! I just wanted to raise these observations for awareness and to clarify the expected local development workflow. |
3 participants
docs/if required so people know about the feature.Changes
Playwright Test Improvements
Faster Tests
Better Local Development
Added Claude Commands
/e2e- Run all tests (OSS + Enterprise)/e2e-oss- Run OSS tests, auto-fix failures, re-run until passing/e2e-ee- Run Enterprise tests, auto-fix failures, re-run until passing/e2e-create- Create a new test following existing patternsNote: You can have the tests repeat to guarantee no flakiness
HTML Reports
e.g. this is a downloaded report, we can access reports of failures as well as videos / interactive traces.
CI/CD Features
GitHub Integration
How did you test this code?