sadpandajoe commented on code in PR #35938:
URL: https://github.com/apache/superset/pull/35938#discussion_r2505222596
##########
.github/workflows/bashlib.sh:
##########
@@ -238,8 +239,26 @@ playwright-run() {
say "::group::Run Playwright tests"
echo "Running Playwright with baseURL: ${PLAYWRIGHT_BASE_URL}"
- npx playwright test auth/login --reporter=github --output=playwright-results
- local status=$?
+ if [ -n "$TEST_PATH" ]; then
+ # Check if there are any test files in the specified path
+ if ! find "playwright/tests/${TEST_PATH}" -name "*.spec.ts" -type f
2>/dev/null | grep -q .; then
+ echo "No test files found in ${TEST_PATH} - skipping test run"
+ say "::endgroup::"
+ kill $flaskProcessId
+ return 0
+ fi
+ echo "Running tests: ${TEST_PATH}"
+ # Set INCLUDE_EXPERIMENTAL=true to allow experimental tests to run
+ export INCLUDE_EXPERIMENTAL=true
+ npx playwright test "${TEST_PATH}" --output=playwright-results
+ local status=$?
+ # Unset to prevent leaking into subsequent commands
+ unset INCLUDE_EXPERIMENTAL
+ else
+ echo "Running all required tests (experimental/ excluded via
playwright.config.ts)"
+ npx playwright test --output=playwright-results
Review Comment:
This suggestion would actually break the intended functionality. Here's why:
**How Playwright reporters work:**
- CLI `--reporter=flag` **overrides** the config completely (doesn't merge)
- Without the CLI flag, **all** reporters from `playwright.config.ts` run
**Current setup (playwright.config.ts lines 48-54):**
```typescript
reporter: process.env.CI ? [
['github'], // ← GitHub Actions annotations ARE active
['list'], // Shows test names during execution
['html'], // Interactive HTML report
['json'], // Machine-readable results
] : [...]
```
**The problem we solved:**
- **Before (with `--reporter=github` on CLI):** Only 'github' reporter ran,
no test names shown during execution
- **After (no CLI override):** All 4 reporters run, including 'github' for
annotations + 'list' for readable output
Commit 7dfca8d70f specifically removed the CLI override to enable multiple
reporters. Restoring `--reporter=github` would revert this fix and lose the
list/html/json reporters.
**Bottom line:** GitHub annotations are still working (via config), and we
now also get better CI output. The CLI flag should remain removed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
