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

Added test-case for deprecated phpstorm stub with patch-version #1406

Closed
wants to merge 3 commits into from

Conversation

staabm
Copy link
Contributor

@staabm staabm commented Mar 13, 2024

tests the ternary in

which wasn't covered before

@Ocramius
Copy link
Member

which wasn't covered before

Was it an escaped / uncovered mutation?

@staabm
Copy link
Contributor Author

staabm commented Mar 13, 2024

Was it an escaped / uncovered mutation?

I don't know. I am not a mutation testing expert.

but I realized that this is a path which wasn't test-covered before while working on the PHPStan integration

@Ocramius
Copy link
Member

I don't know. I am not a mutation testing expert.

I was more wondering if mainline caught it - should be visible in the MT framework output :D

@Ocramius Ocramius closed this Mar 13, 2024
@Ocramius Ocramius reopened this Mar 13, 2024
@Ocramius
Copy link
Member

Ocramius commented Mar 13, 2024

(wrong button - thanks Github for not dong TAB+ENTER correctly)

@Ocramius Ocramius added this to the 6.28.0 milestone Mar 13, 2024
@kukulich
Copy link
Collaborator

28ef29b

So I think there may be some escaped mutants now...

@Ocramius
Copy link
Member

Ocramius commented Mar 13, 2024

FWIW, in mainline:


6) /home/runner/work/BetterReflection/BetterReflection/src/SourceLocator/SourceStubber/PhpStormStubsSourceStubber.php:676    [M] IncrementInteger

--- Original
+++ New
@@ @@
         }
         if (preg_match('#@deprecated\\s+(\\d+)\\.(\\d+)(?:\\.(\\d+)?)$#m', $docComment->getText(), $matches) === 1) {
             $major = $matches[1];
-            $minor = $matches[2];
+            $minor = $matches[3];
             $patch = $matches[3] ?? 0;
             $versionId = sprintf('%d%02d%02d', $major, $minor, $patch);
             if ($this->phpVersion >= $versionId) {


7) /home/runner/work/BetterReflection/BetterReflection/src/SourceLocator/SourceStubber/PhpStormStubsSourceStubber.php:677    [M] DecrementInteger

--- Original
+++ New
@@ @@
         if (preg_match('#@deprecated\\s+(\\d+)\\.(\\d+)(?:\\.(\\d+)?)$#m', $docComment->getText(), $matches) === 1) {
             $major = $matches[1];
             $minor = $matches[2];
-            $patch = $matches[3] ?? 0;
+            $patch = $matches[2] ?? 0;
             $versionId = sprintf('%d%02d%02d', $major, $minor, $patch);
             if ($this->phpVersion >= $versionId) {
                 return true;


8) /home/runner/work/BetterReflection/BetterReflection/src/SourceLocator/SourceStubber/PhpStormStubsSourceStubber.php:677    [M] IncrementInteger

--- Original
+++ New
@@ @@
         if (preg_match('#@deprecated\\s+(\\d+)\\.(\\d+)(?:\\.(\\d+)?)$#m', $docComment->getText(), $matches) === 1) {
             $major = $matches[1];
             $minor = $matches[2];
-            $patch = $matches[3] ?? 0;
+            $patch = $matches[4] ?? 0;
             $versionId = sprintf('%d%02d%02d', $major, $minor, $patch);
             if ($this->phpVersion >= $versionId) {
                 return true;


9) /home/runner/work/BetterReflection/BetterReflection/src/SourceLocator/SourceStubber/PhpStormStubsSourceStubber.php:677    [M] DecrementInteger

--- Original
+++ New
@@ @@
         if (preg_match('#@deprecated\\s+(\\d+)\\.(\\d+)(?:\\.(\\d+)?)$#m', $docComment->getText(), $matches) === 1) {
             $major = $matches[1];
             $minor = $matches[2];
-            $patch = $matches[3] ?? 0;
+            $patch = $matches[3] ?? -1;
             $versionId = sprintf('%d%02d%02d', $major, $minor, $patch);
             if ($this->phpVersion >= $versionId) {
                 return true;


10) /home/runner/work/BetterReflection/BetterReflection/src/SourceLocator/SourceStubber/PhpStormStubsSourceStubber.php:677    [M] IncrementInteger

--- Original
+++ New
@@ @@
         if (preg_match('#@deprecated\\s+(\\d+)\\.(\\d+)(?:\\.(\\d+)?)$#m', $docComment->getText(), $matches) === 1) {
             $major = $matches[1];
             $minor = $matches[2];
-            $patch = $matches[3] ?? 0;
+            $patch = $matches[3] ?? 1;
             $versionId = sprintf('%d%02d%02d', $major, $minor, $patch);
             if ($this->phpVersion >= $versionId) {
                 return true;


11) /home/runner/work/BetterReflection/BetterReflection/src/SourceLocator/SourceStubber/PhpStormStubsSourceStubber.php:677    [M] Coalesce

--- Original
+++ New
@@ @@
         if (preg_match('#@deprecated\\s+(\\d+)\\.(\\d+)(?:\\.(\\d+)?)$#m', $docComment->getText(), $matches) === 1) {
             $major = $matches[1];
             $minor = $matches[2];
-            $patch = $matches[3] ?? 0;
+            $patch = 0 ?? $matches[3];
             $versionId = sprintf('%d%02d%02d', $major, $minor, $patch);
             if ($this->phpVersion >= $versionId) {
                 return true;

That should be OK with this patch now :)

This is mostly to show @staabm how to read this: I think MT is something that will be great for your testing toolbox, since you are so detail-oriented 💪

@Ocramius
Copy link
Member

I don't see mutation changes here: I think your test scenario did not hit the edge case you were going for, as the mutations stayed the same :D

@staabm
Copy link
Contributor Author

staabm commented Mar 13, 2024

thanks for the explanation.

lets see whether we can kill some mainline mutants with nikic/PHP-Parser#985 :-)

@staabm
Copy link
Contributor Author

staabm commented Mar 13, 2024

FWIW, in mainline:

looking at the latest build of the latest 6.28.x-commit, I don't see the mutants you referenced in your comment

https://github.com/Roave/BetterReflection/actions/runs/8259610533/job/22593799759

$stubData->getStub(),
);

if (PHP_VERSION_ID >= 80100) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ahh I think we don't catch a bug because we never reach the ELSE case of this condition (because CI is running only 8.2/8.3)

Copy link
Member

Choose a reason for hiding this comment

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

Ah, good catch! Indeed!

Perhaps we can change the fixture to introduce deprecations with higher/fictional PHP versions? :D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me first make sure phpstan can catch such bugs in the future. Brb

Copy link
Contributor Author

Choose a reason for hiding this comment

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

phpstan/phpstan-src#2968 should help us in the future.

@staabm
Copy link
Contributor Author

staabm commented Mar 17, 2024

superseded by #1408

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

Successfully merging this pull request may close these issues.

3 participants