Add systemd service and timer configuration for continuous Playwright…#2572
Add systemd service and timer configuration for continuous Playwright…#2572vwh wants to merge 6 commits intoquran:playwright-testsfrom
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds systemd-based automated Playwright test infrastructure: a README documenting usage and configuration, a oneshot service unit that runs a specific Playwright test with a Discord reporter, and a timer unit that schedules the service to run every 5 minutes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Timer as systemd.timer
participant Systemd as systemd
participant Service as quran-tests.service
participant Runner as mise (wrapper)
participant Playwright as yarn playwright
participant Reporter as Discord reporter
Note over Timer,Systemd `#E0F7FA`: every 5 minutes (OnCalendar)
Timer->>Systemd: schedule trigger
Systemd->>Service: start oneshot unit
Service->>Runner: run in /srv/apps/playwright-tests
Runner->>Playwright: execute test file
Playwright->>Reporter: send results
Playwright->>Systemd: stdout/stderr -> journal
Systemd->>Timer: unit completes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.*📄 CodeRabbit inference engine (.cursor/rules/bug-handling-with-todo-comments.mdc)
Files:
🧠 Learnings (1)📓 Common learnings🔇 Additional comments (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
tests/service/README.md(1 hunks)tests/service/quran-tests.service(1 hunks)tests/service/quran-tests.timer(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.*
📄 CodeRabbit inference engine (.cursor/rules/bug-handling-with-todo-comments.mdc)
If you encounter a bug in existing code, or the instructions lead to suboptimal or buggy code, add comments starting with "TODO:" outlining the problems.
**/*.*: Utilize Early Returns: Use early returns to avoid nested conditions and improve readability.
Conditional Classes: Prefer conditional classes over ternary operators for class attributes.
**/*.*: Use comments sparingly, and when you do, make them meaningful.
Don't comment on obvious things. Excessive or unclear comments can clutter the codebase and become outdated.
Use comments to convey the 'why' behind specific actions or explain unusual behavior and potential pitfalls.
Provide meaningful information about the function's behavior and explain unusual behavior and potential pitfalls.
**/*.*: Write short functions that only do one thing.
Follow the single responsibility principle (SRP), which means that a function should have one purpose and perform it effectively.
If a function becomes too long or complex, consider breaking it into smaller, more manageable functions.Order functions with those that are composing other functions appearing earlier in the file. For example, if you have a menu with multiple buttons, define the menu function above the buttons.
**/*.*: Always add helpful comments to the code explaining what you are doing.
Never delete old comments, unless they are no longer relevant because the code has been rewritten or deleted.
**/*.*: Choose names for variables, functions, and classes that reflect their purpose and behavior.
A name should tell you why it exists, what it does, and how it is used. If a name requires a comment, then the name does not reveal its intent.
Use specific names that provide a clearer understanding of what the variables represent and how they are used.
Files:
tests/service/quran-tests.servicetests/service/quran-tests.timertests/service/README.md
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: quran/quran.com-frontend-next PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-07T08:37:58.213Z
Learning: Use Playwright for integration tests
📚 Learning: 2025-10-07T08:37:58.213Z
Learnt from: CR
Repo: quran/quran.com-frontend-next PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-07T08:37:58.213Z
Learning: Use Playwright for integration tests
Applied to files:
tests/service/quran-tests.service
🔇 Additional comments (5)
tests/service/quran-tests.timer (1)
1-3: LGTM: Timer configuration is sound.The unit dependencies, persistent setting, and install target are correctly configured. The
Requires=quran-tests.serviceensures the service unit is loaded,Persistent=truewill execute missed runs after system downtime, andWantedBy=timers.targetenables proper systemd timer lifecycle management.Also applies to: 7-7, 9-10
tests/service/quran-tests.service (2)
7-8: Note: Hardcoded deployment paths.The service uses hardcoded paths (
/srv/apps/playwright-tests) which is standard for systemd but limits portability. Ensure this path requirement is documented in deployment procedures or consider using systemd specifiers if the path may vary across environments.Based on the README, this path is already documented. Verify that deployment/installation procedures clearly specify this directory structure requirement.
1-6: LGTM: Service configuration is well-structured.The unit dependencies (
After=network.target), service type (Type=simple), logging configuration (journal output), and timeout/kill settings are appropriately configured for a long-running test execution service. The 180-second start timeout accommodates Playwright's initialization time, and the graceful termination settings allow proper cleanup.Also applies to: 11-12, 14-17
tests/service/README.md (2)
9-9: Inconsistent timer interval documentation.The documentation states the timer triggers every 5 minutes, but the actual
quran-tests.timerhas a critical bug whereOnCalendar=*:*:0/5means every 5 seconds, not minutes. Once the timer configuration is fixed, this documentation will be accurate.This comment relates to the critical issue flagged in
quran-tests.timerline 6. Verify both files are corrected consistently.
20-119: LGTM: Comprehensive setup and management documentation.The setup instructions, service management commands, and log viewing examples are thorough and correctly use systemd user service conventions (
--userflag,~/.config/systemd/user/paths,journalctl --user). The symlink approach keeps the systemd units under version control while installing them properly.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
.gitignore(1 hunks)tests/service/quran-tests.service(1 hunks)tests/service/quran-tests.timer(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.*
📄 CodeRabbit inference engine (.cursor/rules/bug-handling-with-todo-comments.mdc)
If you encounter a bug in existing code, or the instructions lead to suboptimal or buggy code, add comments starting with "TODO:" outlining the problems.
**/*.*: Utilize Early Returns: Use early returns to avoid nested conditions and improve readability.
Conditional Classes: Prefer conditional classes over ternary operators for class attributes.
**/*.*: Use comments sparingly, and when you do, make them meaningful.
Don't comment on obvious things. Excessive or unclear comments can clutter the codebase and become outdated.
Use comments to convey the 'why' behind specific actions or explain unusual behavior and potential pitfalls.
Provide meaningful information about the function's behavior and explain unusual behavior and potential pitfalls.
**/*.*: Write short functions that only do one thing.
Follow the single responsibility principle (SRP), which means that a function should have one purpose and perform it effectively.
If a function becomes too long or complex, consider breaking it into smaller, more manageable functions.Order functions with those that are composing other functions appearing earlier in the file. For example, if you have a menu with multiple buttons, define the menu function above the buttons.
**/*.*: Always add helpful comments to the code explaining what you are doing.
Never delete old comments, unless they are no longer relevant because the code has been rewritten or deleted.
**/*.*: Choose names for variables, functions, and classes that reflect their purpose and behavior.
A name should tell you why it exists, what it does, and how it is used. If a name requires a comment, then the name does not reveal its intent.
Use specific names that provide a clearer understanding of what the variables represent and how they are used.
Files:
tests/service/quran-tests.servicetests/service/quran-tests.timer
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: quran/quran.com-frontend-next PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-07T08:37:58.213Z
Learning: Use Playwright for integration tests
📚 Learning: 2025-10-07T08:37:58.213Z
Learnt from: CR
Repo: quran/quran.com-frontend-next PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-07T08:37:58.213Z
Learning: Use Playwright for integration tests
Applied to files:
tests/service/quran-tests.service
🔇 Additional comments (7)
.gitignore (1)
38-40: LGTM: Appropriate ignore pattern for local configuration.The addition of
.mise.local.tomlto.gitignoreis correct, preventing local mise configuration from being committed to version control.tests/service/quran-tests.service (3)
1-3: LGTM: Appropriate [Unit] configuration.The unit description is clear and
After=network.targetcorrectly ensures network availability before running Playwright tests.
5-7: LGTM: Correct service type and configuration.
Type=oneshotis appropriate for a timer-triggered test service. The absence of aRestart=directive is correct since the timer handles scheduling.
9-13: LGTM: Appropriate logging and timeout configuration.Redirecting output to the journal is correct for systemd services, and the timeout values (180s start, 60s stop) are reasonable for Playwright test execution.
tests/service/quran-tests.timer (3)
1-3: LGTM: Appropriate timer unit configuration.The description is clear and
Requires=quran-tests.servicecorrectly establishes the dependency on the service unit.
5-7: LGTM: Timer correctly schedules every 5 minutes.The
OnCalendar=*-*-* *:0/5:00format is correct for 5-minute intervals (runs at :00, :05, :10, etc.).Persistent=trueensures missed runs execute after system downtime.
9-10: LGTM: Correct installation target.
WantedBy=timers.targetis the appropriate target for systemd timer units.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
This PR adds systemd-based continuous integration testing infrastructure for Quran.com's Playwright tests. The service is configured to run URL path validation tests every 5 minutes, with results reported to Discord via a custom reporter.
Key Changes:
- Systemd timer configured to trigger tests every 5 minutes using
OnCalendarsyntax - Service unit configured to run a specific Playwright test file with Discord reporting
- Comprehensive documentation covering setup, management, and troubleshooting
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| tests/service/quran-tests.timer | Defines the systemd timer that triggers test execution every 5 minutes with persistent scheduling |
| tests/service/quran-tests.service | Configures the oneshot service that executes Playwright tests via mise with timeout and journaling settings |
| tests/service/README.md | Provides complete documentation for setup, service management, status monitoring, and environment configuration |
|
|
||
| [Service] | ||
| Type=oneshot | ||
| WorkingDirectory=/srv/apps/playwright-tests |
There was a problem hiding this comment.
The hardcoded path /srv/apps/playwright-tests is specific to a particular deployment environment. This should be documented as an example that needs to be customized, or consider using a relative path or environment variable to make the service more portable across different deployment environments.
| WorkingDirectory=/srv/apps/playwright-tests | |
| # Set PLAYWRIGHT_TESTS_DIR to the directory containing your Playwright tests. | |
| Environment="PLAYWRIGHT_TESTS_DIR=/srv/apps/playwright-tests" | |
| WorkingDirectory=${PLAYWRIGHT_TESTS_DIR} |
| [Service] | ||
| Type=oneshot | ||
| WorkingDirectory=/srv/apps/playwright-tests | ||
| ExecStart=mise x -- yarn playwright test tests/integration/url/path.spec.ts --reporter=./tests/reporter/discord-reporter.js,list |
There was a problem hiding this comment.
The service will run only a single test file (tests/integration/url/path.spec.ts) rather than the full test suite. If this is intentional for continuous monitoring of critical paths, this should be clearly documented in the README explaining why only this specific test file is run. If the intention is to run all tests, the path should be updated to include all test files or removed to run the entire test suite.
| ExecStart=mise x -- yarn playwright test tests/integration/url/path.spec.ts --reporter=./tests/reporter/discord-reporter.js,list | |
| ExecStart=mise x -- yarn playwright test --reporter=./tests/reporter/discord-reporter.js,list |
| ```toml | ||
| [env] | ||
| PLAYWRIGHT_TEST_BASE_URL = "https://quran.com" | ||
| PLAYWRIGHT_SKIP_WEB_SERVER = "true" | ||
| DISCORD_BOT_TOKEN = "your_bot_token_here" | ||
| DISCORD_CHANNEL_ID = "your_channel_id_here" | ||
| TEST_USER_EMAIL = "test@example.com" | ||
| TEST_USER_PASSWORD = "password" | ||
| TEST_USER_USERNAME = "testuser" | ||
| ``` |
There was a problem hiding this comment.
The environment variables section shows example credentials with placeholder values like "your_bot_token_here" and "password". While this is documentation, it should include a security warning that these values must be kept secret and should never be committed to version control. Consider adding a note like ".mise.local.toml in .gitignore."
| # Navigate to service directory | ||
| cd /srv/apps/playwright-tests/tests/service |
There was a problem hiding this comment.
[nitpick] The hardcoded path /srv/apps/playwright-tests appears multiple times in the documentation. Consider adding a note at the beginning of the Setup section clarifying that this path should be adjusted to match the actual deployment location, or use a placeholder like <PROJECT_ROOT> to make it clearer that this needs customization.
| - Run Playwright tests against the live quran.com site | ||
| - Send Discord notifications for test failures |
There was a problem hiding this comment.
The README states that the service "Run Playwright tests against the live quran.com site" (line 17), but the ExecStart command in the service file only runs a single test file (tests/integration/url/path.spec.ts). The documentation should be updated to clarify that only URL path tests are run, not the entire test suite, or provide guidance on how to customize which tests are executed.
2 participants
Summary by CodeRabbit