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

Add random to core extensions #367

Merged
merged 12 commits into from
Dec 14, 2022
Merged

Conversation

MidnightDesign
Copy link
Contributor

No description provided.

@Ocramius
Copy link
Collaborator

As the test results show, the newly added detail requires a verification :)

@Ocramius Ocramius added this to the 4.4.0 milestone Dec 13, 2022
@MidnightDesign
Copy link
Contributor Author

MidnightDesign commented Dec 13, 2022

As the test results show, the newly added detail requires a verification :)

I'm on it, but I don't know if it will be that easy because PHP <8.2 doesn't know about the extension.

@Ocramius
Copy link
Collaborator

Yeh, will be tricky, especially considering mutation tests run on PHP 8.1 🤔

@MidnightDesign
Copy link
Contributor Author

MidnightDesign commented Dec 13, 2022

@Ocramius Should I run the mutation tests on 8.2 as well? I'm not sure if that can make any difference, but they're already running on 8.0 and 8.1.

@MidnightDesign
Copy link
Contributor Author

Most of the new surviving mutants are probably coming from the Infection upgrade.

The only "real" new one is

--- Original
+++ New
@@ @@
         $definedSymbols = [];
         foreach ($extensionNames as $extensionName) {
             $extensionName = self::ALTERNATIVES[$extensionName] ?? $extensionName;
-            if ($extensionName === 'random' && PHP_VERSION_ID < 80200) {
+            if ($extensionName === 'random' && PHP_VERSION_ID <= 80200) {
                 continue;
             }
             try {

but I'm not sure how to kill that one. Should I just lower the minimum covered MSI?

@Ocramius
Copy link
Collaborator

@MidnightDesign
Copy link
Contributor Author

Done. Another option would be to just use version_compare() instead. I can't imagine a mutation for that.

@Ocramius
Copy link
Collaborator

Had to rollback to phpunit/php-code-coverage:9.2.17, since newer versions are foobar'd.

Ref: sebastianbergmann/php-code-coverage#964

@Ocramius Ocramius self-assigned this Dec 13, 2022
@Ocramius
Copy link
Collaborator

Hmm, score still went down - perhaps we should just revert all composer.lock and composer.json changes for now?

@MidnightDesign
Copy link
Contributor Author

Some of the locked dependencies don't allow 8.2, that's why I upgraded them:

  Problem 1
    - ocramius/package-versions is locked to version 2.4.0 and an update of this package was not requested.
    - ocramius/package-versions 2.4.0 requires php ~8.0.0 || ~8.1.0 -> your php version (8.2.0) does not satisfy that requirement.
  Problem 2
    - phpspec/prophecy is locked to version 1.14.0 and an update of this package was not requested.
    - phpspec/prophecy 1.14.0 requires php ^7.2 || ~8.0, <8.2 -> your php version (8.2.0) does not satisfy that requirement.
  Problem 3
    - roave/infection-static-analysis-plugin is locked to version 1.13.x-dev and an update of this package was not requested.
    - roave/infection-static-analysis-plugin 1.13.x-dev requires php ~7.4.7|~8.0.0|~8.1.0 -> your php version (8.2.0) does not satisfy that requirement.
  Problem 4
    - ocramius/package-versions 2.4.0 requires php ~8.0.0 || ~8.1.0 -> your php version (8.2.0) does not satisfy that requirement.
    - vimeo/psalm v4.15.0 requires composer/package-versions-deprecated ^1.8.0 -> satisfiable by ocramius/package-versions[2.4.0].
    - vimeo/psalm is locked to version v4.15.0 and an update of this package was not requested.

@MidnightDesign
Copy link
Contributor Author

Yay, seems like my last commit did the trick.

Copy link
Collaborator

@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.

Excellent, thanks @MidnightDesign!

@Ocramius Ocramius merged commit 45a6a59 into maglnet:4.4.x Dec 14, 2022
@MidnightDesign MidnightDesign deleted the patch-1 branch December 14, 2022 09:09
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.

2 participants