chore: Add PHPUnit check to CI workflow #2780
Conversation
WalkthroughThe updates introduce a new Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub Actions
participant Node.js
participant npm
participant PHP
participant WP Env
participant PHPUnit
GitHub Actions->>Node.js: Setup Node.js v20
GitHub Actions->>npm: npm install
GitHub Actions->>npm: npm run build
GitHub Actions->>PHP: Setup PHP
GitHub Actions->>npm: npm run phpcs
GitHub Actions->>WP Env: npm run env:start
GitHub Actions->>PHPUnit: npm run phpunit
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Suggested labels
Suggested reviewers
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
.github/workflows/phpcs.yml (2)
11-21: Usenpm cifor reproducible installs & faster caching.
npm iignores the lock-file’s exact versions and is slower.
npm ciis purpose-built for CI, guarantees deterministic installs, and works seamlessly withsetup-node’s built-in cache.- - name: Npm install and build - run: | - npm i - npm run build + - name: Install deps & build + run: | + npm ci + npm run build
52-58: Gracefully shut down Docker containers at job end.
wp-env startleaves containers running until the runner is torn down. While the runner is ephemeral, a tidy shutdown avoids stray logs, eases local reuse of the workflow, and prepares for potential parallel jobs.- name: Running the phpunit tests run: | npm run phpunit + + - name: Stop WordPress Environment + if: always() + run: | + npm run env:stoppackage.json (2)
22-29:test:phpunitscript can race ifwp-env startis still booting.
wp-env startreturns immediately once Docker compose is up, but WP may not yet respond to CLI commands, causing flaky runs locally.
Add--wait=60(or similar) to ensure the environment is ready before invoking PHPUnit.- "test:phpunit": "wp-env start && npm run phpunit && wp-env stop" + "test:phpunit": "wp-env start --wait=60 && npm run phpunit && wp-env stop"
27-28: Quoting the PHPUnit path improves cross-platform safety.Paths containing hyphens or spaces can break when parsed by the shell. Wrapping the path guards against that.
- "phpunit": "wp-env run cli --env-cwd=wp-content/plugins/dokan vendor/bin/phpunit -c phpunit.xml", + "phpunit": "wp-env run cli --env-cwd=wp-content/plugins/dokan \"vendor/bin/phpunit\" -c phpunit.xml",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/phpcs.yml(2 hunks).wp-env.json(1 hunks)package.json(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: e2e tests (3, 3)
- GitHub Check: api tests (1, 1)
- GitHub Check: e2e tests (2, 3)
- GitHub Check: e2e tests (1, 3)
- GitHub Check: Run PHPCS inspection
🔇 Additional comments (1)
.wp-env.json (1)
3-3: Re-evaluate the PHP 7.4 requirement.WordPress currently recommends PHP 8.1+. Running tests on an EOL PHP version diminishes coverage for supported stacks and loses out on performance/security improvements.
If Dokan still supports 7.4, add a matrix to run against both 7.4 and 8.1; otherwise bump this value.
| "core": null, | ||
| "phpVersion": "7.4", | ||
| "plugins": [ |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Pin WordPress Core to a deterministic version.
Leaving "core": null implicitly pulls the latest nightly build every time the workflow runs, which may introduce breaking changes outside your control.
Consider locking to the latest stable (e.g. "core": "6.5"), or at least "core": "latest" to stay on stable releases.
- "core": null,
+ "core": "latest",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "core": null, | |
| "phpVersion": "7.4", | |
| "plugins": [ | |
| "core": "latest", | |
| "phpVersion": "7.4", | |
| "plugins": [ |
🤖 Prompt for AI Agents
In the .wp-env.json file around lines 2 to 4, the "core" property is set to
null, which causes the environment to pull the latest nightly WordPress build on
each run. To fix this, replace null with a specific stable version string like
"6.5" or "latest" to ensure the environment uses a deterministic and stable
WordPress core version.
| "tailwind-merge": "^2.6.0", | ||
| "tailwindcss": "^3.3.3", |
There was a problem hiding this comment.
Remove the duplicated tailwind-merge entry to avoid version conflicts.
The package exists in both devDependencies (^2.6.0) and dependencies (^2.5.5). NPM will dedupe, but the older prod-level entry may override the newer dev version, leading to subtle build issues.
- "tailwind-merge": "^2.5.5",Also applies to: 102-102
🤖 Prompt for AI Agents
In package.json around lines 51-52 and line 102, remove the duplicated
"tailwind-merge" entry from either devDependencies or dependencies to avoid
version conflicts. Keep only one entry with the desired version (preferably the
newer "^2.6.0") and delete the other to ensure consistent package resolution and
prevent subtle build issues.
shohag121
added
Needs: Testing
All Submissions:
Changes proposed in this Pull Request:
Related Pull Request(s)
Closes
How to test the changes in this Pull Request:
Changelog entry
No change log needed.
Before Changes
Describe the issue before changes with screenshots(s).
After Changes
Describe the issue after changes with screenshot(s).
Feature Video (optional)
Link of detailed video if this PR is for a feature.
PR Self Review Checklist:
FOR PR REVIEWER ONLY:
Summary by CodeRabbit
New Features
Chores