-
Notifications
You must be signed in to change notification settings - Fork 297
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Development
: Add different execution modes for local playwright setup
#10064
base: develop
Are you sure you want to change the base?
Conversation
WalkthroughThis pull request focuses on improving the Playwright testing setup and documentation. The changes include updating the README.md file with more detailed and clear instructions for running Playwright tests, modifying the Changes
Sequence DiagramsequenceDiagram
participant Dev as Developer
participant Script as startPlaywrightUI.sh
participant Npm as NPM
participant Playwright as Playwright UI
Dev->>Script: Execute script
Script->>Npm: npm install
Npm-->>Script: Dependencies installed
Script->>Npm: npm run playwright:setup-local
Npm-->>Script: Local setup complete
Script->>Playwright: Open Playwright UI
Playwright-->>Dev: Playwright UI launched
Possibly related PRs
Suggested Labels
Suggested Reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
supporting_scripts/playwright/startPlaywright.sh (1)
15-16
: Consider adding error handling for the test command.While the script uses
set -e
for general error handling, consider adding specific error handling for the test command to provide better feedback on test failures.echo "Run all playwright tests" -npm run playwright:test +if ! npm run playwright:test; then + echo "Playwright tests failed. Check the test output above for details." + exit 1 +fi
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
supporting_scripts/playwright/README.md
(1 hunks)supporting_scripts/playwright/startPlaywright.sh
(1 hunks)supporting_scripts/playwright/startPlaywrightUI.sh
(1 hunks)
🔇 Additional comments (2)
supporting_scripts/playwright/README.md (2)
19-23
: LGTM! Improved code block formatting.
The addition of proper code block formatting for the setupUsers.sh
command improves readability and follows Markdown best practices.
26-41
: LGTM! Clear and well-structured documentation.
The documentation improvements effectively:
- Separate and explain different execution modes
- Provide clear distinctions between UI mode and headless testing
- Include helpful details about features available in each mode
#!/bin/sh | ||
|
||
set -e | ||
|
||
artemis_path="$(readlink -f "$(dirname $0)/../..")" | ||
|
||
echo "Installing Playwright and dependencies" | ||
|
||
cd "$artemis_path/src/test/playwright" | ||
|
||
npm install | ||
|
||
npm run playwright:setup-local || true | ||
|
||
echo "Start Playwright in UI mode" | ||
npm run playwright:open |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider reducing code duplication with a common setup script.
There's significant code duplication between startPlaywright.sh
and startPlaywrightUI.sh
. Consider extracting common setup logic into a shared script.
Create a new file setupPlaywright.sh
:
#!/bin/sh
set -e
artemis_path="$(readlink -f "$(dirname $0)/../..")"
cd "$artemis_path/src/test/playwright"
# Check if playwright is already installed
if [ ! -d "node_modules" ]; then
echo "Installing Playwright and dependencies"
npm install
npm run playwright:setup-local || true
fi
Then modify both scripts to source this common setup:
#!/bin/sh
set -e
-artemis_path="$(readlink -f "$(dirname $0)/../..")"
-
-echo "Installing Playwright and dependencies"
-
-cd "$artemis_path/src/test/playwright"
-
-npm install
-
-npm run playwright:setup-local || true
+source "$(dirname $0)/setupPlaywright.sh"
echo "Start Playwright in UI mode"
npm run playwright:open
Checklist
General
Motivation and Context
With the simplified playwright setup, the user should be able to run all to run all tests.
Description
Adds another script, to be able to run either all tests, or run Playwright in UI mode. Also extends the README explaining the two options.
Steps for Testing
Testserver States
Note
These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Click on the badges to get to the test servers.
Review Progress
Code Review
Manual Tests
Summary by CodeRabbit
New Features
startPlaywrightUI.sh
).Bug Fixes
startPlaywright.sh
to run all Playwright tests instead of launching the UI.Documentation