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

#599 add infection (mutation testing) to test runs #606

Merged
merged 12 commits into from
May 28, 2020

Conversation

Ocramius
Copy link
Member

Fixes #599

… exception

This failure was highlighted by random test execution order: hard to spot otherwise
…ed AST nodes

Before this change, the `MemoizingParser` would keep the AST in memory as-is, but
`nikic/php-parser`'s AST is mutable, and our tests do mutate that state.

Running `vendor/bin/phpunit --random-order-seed=1590604675 --order-by=random` does
highlight a number of failures caused by tests modifying the AST and tripping on
each other.

By making the memoization referentially transparent, we get rid of this huge design
issue completely at the cost of some performance.
@Ocramius Ocramius added enhancement dependencies Pull requests that update a dependency file labels May 27, 2020
@Ocramius Ocramius added this to the 4.3.0 milestone May 27, 2020
@Ocramius Ocramius self-assigned this May 27, 2020
…files

If, for some reason, files are no longer accessible after having been included by the PHP
process, then the `AutoloadSourceLocator` is supposed to skip them when looking for declared
constants.
…HP 7.2 for good

There's no point in trying to keep compatibility with multiple infection versions: killing the
support for PHP 7.2 instead.

This also means that this particular change will not land in `4.3.0`, but in a later release
instead.

Locking `composer.json` platform to PHP 7.3.0 to be sure that we don't update dependencies
against something that is too new (most developers on the project are already on PHP 7.4.6+)
…lode them manually

This leads to a lot of repetitive, but all-equal YAML, easy to split across multiple files
and to compare later on.

Ref: https://github.community/t/support-for-yaml-anchors/16128/33
… very-similar files

While this is repetitive, it allows for quick introspection to check if they are still
the same, or whether they need updating. Individual differences become much more evident,
if you don't have to scroll through thousands of lines of YAML garbage.

Ref: https://github.community/t/support-for-yaml-anchors/16128/33
…ed to look for constants

We were using `is_readable()` before, but that's a bit fragile: we've already written a well-tested
utility to verify file accessibility, so we use that instead.

Ref: #606 (comment)
Windows images used by github actions don't seem to understand `php -dmemory_limit=-1`, possibly being
escaped somehow: working around the issue by removing the limit in the base configuration of PHP itself.
@@ -0,0 +1,62 @@
# https://help.github.com/en/categories/automating-your-workflow-with-github-actions
Copy link
Member Author

Choose a reason for hiding this comment

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

My eyes were spinning too much, so I split the workflows into multiple files that I could compare with diff without going bananas.

Tried using anchors, but they aren't supported by GitHub's server-side parser.

@Ocramius Ocramius modified the milestones: 4.3.0, 4.4.0 May 27, 2020
@@ -3,7 +3,7 @@
"description": "Better Reflection - an improved code reflection API",
"license": "MIT",
"require": {
"php": ">=7.2.0,<7.5.0",
"php": ">=7.3.0,<7.5.0",
Copy link
Member Author

Choose a reason for hiding this comment

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

PHP 7.2.0 is gone: it isn't supported by infection/infection, so I'd rather have it gone, and have this target 4.4.0 of this repo instead.

@Ocramius
Copy link
Member Author

Ocramius commented May 27, 2020

@ondrejmirtes I got this interesting error with this new patch:

  -- --------------------------------------------------------------------------- 
     Error                                                                      
 -- --------------------------------------------------------------------------- 
     Reached internal errors count limit of 50, exiting...                      
     Internal error: Child process timed out after 60.0 seconds. Try making it  
     longer with parallel.processTimeout setting.                               
 -- ---------------------------------------------------------------------------

Do you have a clue on what may be going on? It's running the phar version (0.12.25), which didn't change with the patch itself.

EDIT: looks like this was a hiccup 🤔

…ster` or pull requests

Pushes are not going to be relevant to us unless a pull request is opened against upstream.
@ondrejmirtes
Copy link
Contributor

I sent #609. This might have been a temporary hiccup, but this file should be excluded in the current stable version. Anyway. On my computer and on @kukulich's the analysis doesn't finish even with a lot of RAM and with this file excluded it doesn't have any problems.

@Ocramius
Copy link
Member Author

Gonna cut 4.3.0 as soon as I have a moment, will then merge and release with this too 👍

.github/workflows/psalm.yml Outdated Show resolved Hide resolved
Co-authored-by: Alexander Berl <a.berl@outlook.com>
kukulich pushed a commit to kukulich/BetterReflection that referenced this pull request May 28, 2020
…ed to look for constants

We were using `is_readable()` before, but that's a bit fragile: we've already written a well-tested
utility to verify file accessibility, so we use that instead.

Ref: Roave#606 (comment)
@Ocramius
Copy link
Member Author

4.3.0 is out: this can now be merged \o/

@Ocramius Ocramius merged commit 98870a8 into master May 28, 2020
@Ocramius Ocramius deleted the feature/#599-add-infection-to-test-runs branch May 28, 2020 14:16
DanielBadura pushed a commit to DanielBadura/BetterReflection that referenced this pull request Jul 2, 2020
…ed to look for constants

We were using `is_readable()` before, but that's a bit fragile: we've already written a well-tested
utility to verify file accessibility, so we use that instead.

Ref: Roave#606 (comment)
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.

Add infection with minimum mutation score indicator to test runs
5 participants