Conversation
|
Hello! 👋 Thanks for opening this pull request! Please check out our contributing guidelines. We appreciate you taking the initiative to contribute to this project. Contributing isn't limited to just code. We encourage you to contribute in the way that best fits your abilities, by writing tutorials, giving a demo at your local meetup, helping other users with their support questions, or revising our documentation. Here are some useful Composer commands to get you started:
To run a single Behat test, you can use the following command: # Run all tests in a single file
composer behat features/some-feature.feature
# Run only a specific scenario (where 123 is the line number of the "Scenario:" title)
composer behat features/some-feature.feature:123You can find a list of all available Behat steps in our handbook. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request improves cross-platform compatibility for the test suite, specifically targeting Windows environments. Key changes include removing a PHP tag from a file content check, skipping Unix-specific scenarios on Windows, and introducing a dynamic {DIRECTORY_SEPARATOR} variable for path assertions. Feedback points out a syntax error in a PowerShell command where an en-dash was used instead of a hyphen and suggests refactoring repeated setup steps into a Background section to reduce duplication.
Co-authored-by: Pascal Birchler <pascal.birchler@gmail.com>
There was a problem hiding this comment.
Pull request overview
Updates the Behat feature tests to be more compatible with Windows environments by avoiding Unix-specific assumptions in a few scenarios.
Changes:
- Split the “Adds copyright comments” scenario into Windows and non-Windows variants using OS-specific tags/commands.
- Make debug-output path assertions OS-agnostic by using
DIRECTORY_SEPARATOR. - Relax a
make-phpcontent assertion to avoid line-ending related failures.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
features/makepot.feature |
Adds Windows-specific scenario coverage and normalizes path assertions for Windows path separators. |
features/makephp.feature |
Adjusts expected generated PHP output to reduce sensitivity to platform line-ending differences. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @require-windows | ||
| Scenario: Adds copyright comments | ||
| When I run `wp scaffold plugin hello-world` |
There was a problem hiding this comment.
There are now two scenarios with the exact same name (one Windows-only, one non-Windows). Even if the runner allows it, it makes test reports and reruns ambiguous. Rename one/both scenarios to include the platform (or avoid duplication entirely) so failures can be identified unambiguously.
| And the foo-plugin/foo-plugin-de_DE.l10n.php file should contain: | ||
| """ | ||
| <?php | ||
| return ['domain'=>NULL,'plural-forms'=>'nplurals=2; plural=n != 1;','language'=>'de_DE','project-id-version'=>'Development (5.2.x)','pot-creation-date'=>'','po-revision-date'=>'2019-03-28 19:42+0300','x-generator'=>'Poedit 2.2.1','messages'=>['html_lang_attribute'=>'de-DE','text directionltr'=>'ltr','number_format_decimal_point'=>',','number_format_thousands_sep'=>'.','Update %s now'=>'Jetzt %s aktualisieren','[%1$s] Confirm Action: %2$s'=>'[%1$s] Aktion bestätigen: %2$s','[%s] Erasure Request Fulfilled'=>'[%s] Löschauftrag ausgeführt','[%s] Personal Data Export'=>'[%s] Export personenbezogener Daten']]; |
There was a problem hiding this comment.
Dropping the <?php line from the expected content makes this assertion much less strict and could let a regression slip through (e.g., missing PHP open tag) just to work around line-ending differences. A more robust approach is to assert <?php and the return [...] payload separately (or use an assertion that’s tolerant of CRLF between lines) so Windows compatibility doesn’t reduce coverage.
| @@ -112,6 +113,20 @@ Feature: Generate a POT file of a WordPress project | |||
| # This file is distributed under the same license as the Hello World plugin. | |||
| """ | |||
|
|
|||
| @require-windows | |||
| Scenario: Adds copyright comments | |||
| When I run `wp scaffold plugin hello-world` | |||
| And I run `get-date -f yyyy` | |||
| Then STDOUT should not be empty | |||
| And save STDOUT as {YEAR} | |||
There was a problem hiding this comment.
These tests still rely on platform-specific shell commands to determine the current year (date vs get-date), which duplicates the scenario and can be brittle depending on which shell is used to execute I run on Windows. Consider replacing both with a single cross-platform WP-CLI call (e.g., wp eval emitting date('Y')) so the scenario can run unconditionally without OS-specific tags/duplication.
No description provided.