-
Notifications
You must be signed in to change notification settings - Fork 25
fix: issues that cause tests to fail #400
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
Conversation
* [`brace-expansion` Regular Expression Denial of Service vulnerability](GHSA-v6h2-p8h4-qcjw) * [Uncontrolled resource consumption in `braces`](GHSA-grv7-fg5c-xmjg) (high severity) * [Regular Expression Denial of Service (ReDoS) in `micromatch`](GHSA-952p-6rrq-rcjv) (medium severity)
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.
Pull Request Overview
This PR addresses multiple test failures by fixing type-related issues, updating dependencies, and improving code consistency. The changes focus on resolving PHP type compatibility issues and modernizing the test suite.
- Updates function parameter types and return type hints for better PHP compatibility
- Modernizes test method visibility and fixes logical operators
- Updates Composer dependencies to use more flexible version constraints
Reviewed Changes
Copilot reviewed 12 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| includes/wp-cli/class-orchestrate-sites.php | Fixes integer division and logical operator issues |
| includes/class-singleton.php | Updates return type annotation from self to static |
| includes/class-events-store.php | Adds nullable type hints for optional string parameters |
| composer.json | Updates dependency versions to use flexible constraints and reorganizes configuration |
| tests/unit-tests/*.php | Makes test methods public and improves code consistency |
| tests/bootstrap.php | Changes include statements to include_once for better reliability |
Comments suppressed due to low confidence (1)
tests/unit-tests/test-event.php:73
- Using
assertEqualsWithDeltainstead ofassertEqualsfor timestamp comparison is a good improvement as it accounts for minor timing variations during test execution.
$this->assertEqualsWithDelta( time() + HOUR_IN_SECONDS, $event->get_timestamp(), 1 );
This pull request introduces several improvements to the codebase, with a primary focus on enhancing test code quality and consistency. Key changes include replacing
requireandincludewith their_oncevariants for improved safety, updating test functions to usepublicvisibility for adherence to PHPUnit conventions, and refining test assertions for greater accuracy.Test Code Improvements:
Updated all test functions to use
publicvisibility in compliance with PHPUnit's requirements. This change affects multiple test files, such astest-cli-orchestrate-sites.php,test-event.php,test-events-store.php,test-events.php, andtest-internal-events.php. [1] [2] [3] [4] [5]Improved assertions for test accuracy, such as replacing
assertEqualswithassertEqualsWithDeltain time-sensitive tests to account for minor timing differences.Dependency Loading Enhancements:
requireandincludewithrequire_onceandinclude_oncein__tests__/bootstrap.phpto prevent potential issues with multiple inclusions of the same files. [1] [2]Minor Refactoring:
test-events-store.php. [1] [2]Fixes: #399