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

Fix php version dependent constant deprecation #1408

Merged
merged 3 commits into from
Mar 18, 2024
Merged

Conversation

staabm
Copy link
Contributor

@staabm staabm commented Mar 17, 2024

the previous test was running against PHP 8.1, which we don't run in CI.
thats the reason why it did not catch the implementation bug

@staabm staabm marked this pull request as ready for review March 17, 2024 08:16
@staabm staabm changed the title Fix php version dependent test Fix php version dependent constant deprecation Mar 17, 2024
@staabm
Copy link
Contributor Author

staabm commented Mar 17, 2024

I am afk. Will fix it later today

\define('FILTER_SANITIZE_STRING', 513);
EOT;
$stubData = new StubData($exampleStub, 'filter');
$stubData = $this->sourceStubber->generateConstantStub('MT_RAND_PHP');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am using a real constant now. using faked data the test did not cover reality

@@ -664,14 +679,14 @@ private function isCoreExtension(string $extension): bool
return in_array($extension, self::CORE_EXTENSIONS, true);
}

private function isDeprecatedByPhpDocInPhpVersion(Node\Stmt\Const_ $node): bool
private function isDeprecatedByPhpDocInPhpVersion(Node\Expr\FuncCall $node): bool
Copy link
Contributor Author

@staabm staabm Mar 17, 2024

Choose a reason for hiding this comment

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

in phpstorm stubs we use define('x', 1) and not const $x=1

Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

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

Indeed, the infection report looks:

-4164 mutations were generated:
+4170 mutations were generated:
-    3315 mutants were killed
+    3333 mutants were killed
       0 mutants were configured to be ignored
-      14 mutants were not covered by tests
+       2 mutants were not covered by tests
      10 covered mutants were not detected
-       7 errors were encountered
+       6 errors were encountered
       0 syntax errors were encountered
      14 time outs were encountered
-     804 mutants required more time than configured
+     805 mutants required more time than configured

@Ocramius Ocramius self-assigned this Mar 18, 2024
@Ocramius Ocramius added enhancement dependencies Pull requests that update a dependency file labels Mar 18, 2024
@Ocramius Ocramius added this to the 6.28.0 milestone Mar 18, 2024
@Ocramius
Copy link
Member

Thanks @staabm!

@Ocramius Ocramius merged commit cf4e555 into Roave:6.28.x Mar 18, 2024
23 checks passed
@staabm staabm deleted the fix-test branch March 18, 2024 17:40
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.

2 participants