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

Support BetterReflection 4 #248

Merged
merged 13 commits into from
Jun 22, 2020
Merged

Conversation

Jean85
Copy link
Contributor

@Jean85 Jean85 commented Jun 19, 2020

Fixes #234, includes #247

@Ocramius
Copy link
Member

Ocramius commented Jun 19, 2020 via email

@Jean85 Jean85 force-pushed the support-better-reflection-4 branch from 931870b to c804c2c Compare June 19, 2020 20:23
@Jean85
Copy link
Contributor Author

Jean85 commented Jun 19, 2020

7.3 should probably be removed from the build

Why?

BTW rebased on top of green build in #247, now this should be green too.

@Jean85 Jean85 marked this pull request as ready for review June 19, 2020 20:25
@Ocramius
Copy link
Member

If this is green, it can go in as-is 👍

@Ocramius Ocramius self-assigned this Jun 19, 2020
@Ocramius Ocramius added dependencies Pull requests that update a dependency file enhancement labels Jun 19, 2020
@Ocramius Ocramius added this to the 5.0.0 milestone Jun 19, 2020
@Ocramius
Copy link
Member

I think we should only keep BetterReflection 4.6+ here: that's what the mutation tests seem to also pick up here (although I don't see the mutant from my phone)

@Jean85
Copy link
Contributor Author

Jean85 commented Jun 20, 2020

--show-mutants is missing in CI, I'll add it:

Escaped mutants:
================


1) /home/alai/workspace/OSS/BackwardCompatibilityCheck/src/DetectChanges/BCBreak/PropertyBased/PropertyDocumentedTypeChanged.php:39    [M] UnwrapArrayUnique

--- Original
+++ New
@@ @@
             return Changes::empty();
         }
         try {
-            $fromTypes = array_unique($fromProperty->getDocBlockTypeStrings());
+            $fromTypes = $fromProperty->getDocBlockTypeStrings();
             $toTypes = array_unique($toProperty->getDocBlockTypeStrings());
         } catch (InvalidArgumentException $failedToParseDocblock) {
             // @TODO #134 improve docblock parsing upstream to remove this generic try-catch


2) /home/alai/workspace/OSS/BackwardCompatibilityCheck/src/DetectChanges/BCBreak/PropertyBased/PropertyDocumentedTypeChanged.php:40    [M] UnwrapArrayUnique

--- Original
+++ New
@@ @@
         }
         try {
             $fromTypes = array_unique($fromProperty->getDocBlockTypeStrings());
-            $toTypes = array_unique($toProperty->getDocBlockTypeStrings());
+            $toTypes = $toProperty->getDocBlockTypeStrings();
         } catch (InvalidArgumentException $failedToParseDocblock) {
             // @TODO #134 improve docblock parsing upstream to remove this generic try-catch
             return Changes::empty();

I'll try to fix them...

[EDIT] It seems that the uniqueness applied to those arrays is already done by BetterReflection, is it right? Is it possible to ignore those mutants?

@@ -11,6 +11,12 @@
"mutators": {
"@default": true,
"IdenticalEqual": false,
"NotIdenticalNotEqual": false
"NotIdenticalNotEqual": false,
"UnwrapArrayUnique": {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should likely have tests, not be unwrapped...

@Ocramius
Copy link
Member

It seems that the uniqueness applied to those arrays is already done by BetterReflection, is it right? Is it possible to ignore those mutants?

If the uniqueness isn't needed, then it should likely be removed: mutation testing works both ways, finding unused code or untested code :-)

@Jean85
Copy link
Contributor Author

Jean85 commented Jun 22, 2020

Ok 👍 Since I don't know at which point that mutant will start appearing, I bumped BetterReflection to 4.3, which seems to be the latest minor supporting 7.3, and it doesn't mutate.

@Ocramius
Copy link
Member

Excellent! \o/

Thanks @Jean85, you basically did what I was supposed to do :D

@Ocramius Ocramius merged commit d64ffb9 into Roave:master Jun 22, 2020
@Jean85 Jean85 deleted the support-better-reflection-4 branch June 22, 2020 07:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support roave/better-reflection ^4.0
2 participants