Enable Windows test compat #151
Conversation
Because the test results will change in environments where assert is not valid (such as Windows),
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 Walkthrough📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces modifications to the test methods in the Changes
Possibly related PRs
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
CodeRabbit Configuration File (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
tests/DonutQueryInterceptorPurgeTest.php (1)
54-55: Fix typo in variable namepuregeResultThe change to store the purge result before assertion is good for debugging and test clarity. However, there's a typo in the variable name.
- $puregeResult = $this->repository->purge(new Uri('page://self/html/comment')); - assert($puregeResult); + $purgeResult = $this->repository->purge(new Uri('page://self/html/comment')); + assert($purgeResult);tests/DonutRepositoryTest.php (1)
98-99: Consider using PHPUnit assertion for consistency.While storing the result improves readability, using
assert()here differs from the assertion style used elsewhere in the test class. Consider using$this->assertTrue($commentResult)for consistency with other test assertions.- $commentResult = $queryRepository->purge(new Uri('page://self/html/comment')); - assert($commentResult); + $commentResult = $queryRepository->purge(new Uri('page://self/html/comment')); + $this->assertTrue($commentResult);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
tests/DonutQueryInterceptorPurgeTest.php(1 hunks)tests/DonutRepositoryTest.php(3 hunks)
🔇 Additional comments (3)
tests/DonutRepositoryTest.php (3)
66-67: LGTM! Good improvement in test readability.
Storing the purge result in a variable before assertion improves test clarity and debugging capabilities while maintaining the same functional verification.
115-116: LGTM! Consistent with the improved pattern.
The change follows the same good practice of storing the purge result and uses the appropriate PHPUnit assertion method.
66-67: Verify if these changes resolve Windows-specific test issues.
The pattern of storing purge results before assertions has been consistently applied across multiple test methods. While this improves test clarity, please verify if these changes specifically address any Windows-related test failures, such as timing or race conditions.
Also applies to: 98-99, 115-116
Replaced recursive directory traversal with array_map to delete .php files in the temporary directory. This change reduces the complexity and improves the readability of the cleanup process.
Removed version check for PHP 8 to always set AttributeReader. This enhances code readability and ensures consistent behavior across all PHP versions.
No description provided.