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

Infection does not run any tests if --filter is too big #1545

Open
maks-rafalko opened this issue Aug 7, 2021 · 2 comments · Fixed by #1548
Open

Infection does not run any tests if --filter is too big #1545

maks-rafalko opened this issue Aug 7, 2021 · 2 comments · Fixed by #1548
Assignees
Labels

Comments

@maks-rafalko
Copy link
Member

In #1539, we've added --only-covering-test-cases option that adds --filter='/very_long|regular_expression_for_all_covering_tests/'.

If the mutated line covered by enormous number of tests, the --filter option becomes too large and as a result, we have

Test cache cleared
  PHPUnit 9.5.7 by Sebastian Bergmann and contributors.
  
  No tests executed!

Why No tests executed!? Because here

https://github.com/sebastianbergmann/phpunit/blob/d599f6e25cc939f13c6dcfe00028b6e8d275ad55/src/Runner/Filter/NameFilterIterator.php#L70

in PHPUnit's code, we have a @preg_match with silencing errors via @, that in reality produces the following warning:

Warning: preg_match(): Compilation failed: regular expression is too large at offset 34341 in vendor/phpunit/phpunit/src/Runner/Filter/NameFilterIterator.php on line 79

  • I don't see any working options to extend this limit
  • We can think about not adding all of the data sets from the test case with data providers, but combine them in 1
    Example:
    instead of App\\Tests\\SomeTest\:\:test_case with data set "#1"|App\\Tests\\SomeTest\:\:test_case with data set "#2"" we can just add App\\Tests\\SomeTest\:\:test_case. In the worst case, we will run not covering data sets from the big dataprovider, but this is a working tradeoff
  • We should not treat No tests executed! as a killing mutant (will do in a separate PR)
@maks-rafalko
Copy link
Member Author

Additional ideas how to improve this:

from sanmai:

  • We can have several optimization strategies for --filter. For example, we can go the way you propose here, or we can use the original approach, or we can go test_case_method1|test_case_method2.

  • Next we can run a cost/benefit analysis for each of the strategies. For example, if the original strategy leads to a regex shorter than, say, 300 characters, we could probably just use it and not consider anything else.

  • We know how much time it takes to run every test, and we can tell how much time it would take to run the most optimal set of tests. Next we can check if there's a substantial difference between different remaining strategies. For example, if the "shortest" strategy such as test_case_method1|test_case_method2 leads to a test duration within 1% of the optimal time, we can probably just use the shortest.

  • If we didn't find the optimal strategy, we can optimize by finding a strategy with the shortest resulting regex (we can probably predict its length) that has the least amount of the deviation from the optimum.

  • If we didn't find a suitable strategy and, for suitable strategies, the assumed length is longer than 34000 characters, we can forfeit the approach.

from theofidry:

maybe another patch would be to simply remove --filter and add a warning in the case where the length could not be reduced enough. It will result in a big performance degradation in those cases but IMO it is better than a failure/false result

@theofidry
Copy link
Member

theofidry commented Sep 8, 2021

Alternative idea: see if it's possible to have this setting within PHPUnit config, which although for the regular user does not make much sense, it would fix our issue perfectly.

Another idea would be to split the PHPUnit, i.e. instead of doing:

vendor/bin/phpunit -c config.xml --filter A|B|C

We could do:

vendor/bin/phpunit -c config.xml --filter A \
  && vendor/bin/phpunit -c config.xml --filter B \
  && vendor/bin/phpunit -c config.xml --filter C

However this would likely result in side-effects on how we process/report things afterwards

@maks-rafalko maks-rafalko removed this from the 0.25.0 milestone Jan 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants