Copilot commented on code in PR #35938:
URL: https://github.com/apache/superset/pull/35938#discussion_r2492177635
##########
.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
Review Comment:
The find command will fail if the directory doesn't exist, and the error is
suppressed with `2>/dev/null`. This means if TEST_PATH points to a non-existent
directory, the check will pass and silently skip tests. Consider adding an
explicit directory existence check before the find command: `if [ ! -d
\"playwright/tests/${TEST_PATH}\" ]; then`
```suggestion
if [ -n "$TEST_PATH" ]; then
# Check if the test directory exists
if [ ! -d "playwright/tests/${TEST_PATH}" ]; then
echo "::error::Test directory playwright/tests/${TEST_PATH} does not
exist - skipping test run"
say "::endgroup::"
kill $flaskProcessId
return 1
fi
```
##########
.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:
Missing the `--reporter=github` flag that was present in the original code.
This flag is important for CI environments to generate GitHub Actions
annotations for test failures. Line 259 also has the same issue.
```suggestion
npx playwright test "${TEST_PATH}" --output=playwright-results
--reporter=github
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 --reporter=github
```
--
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]
