Fix webkit frontend tests silently passing when they fail#7408
Merged
JohnMcLear merged 2 commits intoMar 31, 2026
Conversation
Continue processing remaining plugins when one fails instead of crashing. Add summary at the end showing succeeded/failed/skipped counts and plugin names. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove `|| true` from the webkit Playwright test step that was swallowing non-zero exit codes, causing the workflow to always report success regardless of test results. Fixes ether#7405 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Review Summary by QodoFix webkit tests failure reporting and improve plugin workflow resilience
WalkthroughsDescription• Remove || true from webkit test step to properly report failures • Improve update-plugins workflow to continue on individual plugin failures • Add summary output showing succeeded/failed/skipped plugin counts Diagramflowchart LR
A["webkit test step"] -->|remove || true| B["proper exit code handling"]
B -->|test fails| C["workflow fails correctly"]
D["plugin update workflow"] -->|track outcomes| E["succeeded/failed/skipped"]
E -->|continue on error| F["process all plugins"]
F -->|display summary| G["final report"]
File Changes1. .github/workflows/frontend-tests.yml
|
Code Review by Qodo
1. checkPlugin failure ignored
|
Member
Author
|
This should fail on testing because tests were failing but the test runner was ignoring failed tests 😮💨 I'll work on the bugs that make the tests fail next... |
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Comment on lines
+64
to
+69
| if cd etherpad-lite/bin && pnpm run checkPlugin "$plugin" autopush 2>&1; then | ||
| succeeded="$succeeded $plugin" | ||
| else | ||
| echo "WARN: checkPlugin failed for $plugin" | ||
| failed="$failed $plugin" | ||
| fi |
There was a problem hiding this comment.
1. checkplugin failure ignored 📎 Requirement gap ≡ Correctness
The updated update-plugins.yml step continues (and ultimately succeeds) even when `pnpm run checkPlugin` fails, which can mask real CI failures. This violates the requirement that CI should not report success when test/check steps fail.
Agent Prompt
## Issue description
The workflow collects `failed` plugins but never exits non-zero when failures occurred, so the job can report success even when `pnpm run checkPlugin` fails.
## Issue Context
This violates the compliance requirement that CI must fail the workflow when tests/checks fail.
## Fix Focus Areas
- .github/workflows/update-plugins.yml[64-79]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+63
to
70
| # Run checkPlugin with autopush — continue on failure | ||
| if cd etherpad-lite/bin && pnpm run checkPlugin "$plugin" autopush 2>&1; then | ||
| succeeded="$succeeded $plugin" | ||
| else | ||
| echo "WARN: checkPlugin failed for $plugin" | ||
| failed="$failed $plugin" | ||
| fi | ||
| cd ../.. |
There was a problem hiding this comment.
2. Wrong cd after failure 🐞 Bug ☼ Reliability
In update-plugins.yml, cd etherpad-lite/bin is executed inside an if condition, but cd ../.. is executed unconditionally afterward. If the cd etherpad-lite/bin fails, the script will still run cd ../.. from the wrong directory, causing subsequent plugin iterations to run from an unexpected working directory.
Agent Prompt
### Issue description
`update-plugins.yml` conditionally runs `cd etherpad-lite/bin` inside an `if`, but then unconditionally runs `cd ../..`. If the `cd` fails, the unconditional `cd ../..` runs from the wrong directory and breaks subsequent loop iterations.
### Issue Context
This workflow iterates over many plugins; once the working directory is wrong, later `git clone`, `git pull`, and subsequent `cd etherpad-lite/bin` operations can cascade into repeated failures or operating in an unintended path.
### Fix Focus Areas
- .github/workflows/update-plugins.yml[63-71]
### Suggested change
Run the checkPlugin command in a subshell so directory changes do not affect the outer script, for example:
```bash
if (cd etherpad-lite/bin && pnpm run checkPlugin "$plugin" autopush 2>&1); then
succeeded="$succeeded $plugin"
else
echo "WARN: checkPlugin failed for $plugin"
failed="$failed $plugin"
fi
```
Then remove the trailing `cd ../..` (or replace it with an explicit `cd` to a known base directory such as `cd "$GITHUB_WORKSPACE/.."`).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
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.
Summary
|| truefrom the webkit Playwright test step infrontend-tests.ymlTest plan
Fixes #7405
🤖 Generated with Claude Code