Skip to content
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

7.2.5 breaks BC ! #3176

Closed
keradus opened this issue Jun 21, 2018 · 4 comments
Closed

7.2.5 breaks BC ! #3176

keradus opened this issue Jun 21, 2018 · 4 comments

Comments

@keradus
Copy link
Contributor

keradus commented Jun 21, 2018

Q A
PHPUnit version 7.2.5
PHP version 7.1, for example
Installation Method Composer, but doesn't matter

newly released 7.2.5 breaks BC:
bcb4c78#diff-9ae7a972d07df5f73629d5d315bf405aR521

formal declaration of method changed:

-    public static function assertNotEquals($expected, $actual, string $message = '', $delta = 0.0, $maxDepth = 10, $canonicalize = false, $ignoreCase = false): void
+    public static function assertNotEquals($expected, $actual, string $message = '', float $delta = 0.0, int $maxDepth = 10, bool $canonicalize = false, bool $ignoreCase = false): void

for that, my integration crashes on incompatible method headers:
https://travis-ci.org/FriendsOfPHP/PHP-CS-Fixer/jobs/394945074#L655

PHP Fatal error:  Uncaught Declaration of
PHPUnitGoodPractices\Traits\IdentityOverEqualityTrait::assertNotEquals($expected, $actual, string $message = '', $delta = 0, $maxDepth = 10, $canonicalize = false, $ignoreCase = false): void
should be compatible with
PHPUnit\Framework\Assert::assertNotEquals($expected, $actual, string $message = '', float $delta = 0, int $maxDepth = 10, bool $canonicalize = false, bool $ignoreCase = false): void
@sebastianbergmann
Copy link
Owner

Why do people overwrite PHPUnit's assertions? sigh

@keradus
Copy link
Contributor Author

keradus commented Jun 21, 2018

short story? because they are not marked as final

long story? vide https://github.com/PHPUnitGoodPractices/Traits/blob/master/src/IdentityOverEqualityTrait.php#L46-L56 for my concrete case

@sebastianbergmann
Copy link
Owner

I shall make them final in PHPUnit 8 then :-)

@keradus
Copy link
Contributor Author

keradus commented Jun 21, 2018

Then, can we first bring from dead topic of registering assertions/expectations? So one can decide to have some extra assertions available globally (without manual import in every single test) or not register all built-in ones ?
But in general, 👍 for final methods/classes/exposing interfaces over concrete implementations

Also, let me request this once more ;) please, let us start using @internal tag for things out of BC promise, that shall not be use out of this library.
(Eg, I can prepare PR that would make everything marked internal, and then we would open what shall be public)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants