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

Probem when using Infection with bypass-finals #1275

Closed
vitormelon opened this issue Jun 19, 2020 · 7 comments · Fixed by #1728
Closed

Probem when using Infection with bypass-finals #1275

vitormelon opened this issue Jun 19, 2020 · 7 comments · Fixed by #1728

Comments

@vitormelon
Copy link

All mutations were escaping when i was running infection and i didn't know why. I did manually some mutations and the test failed like it should to kill de mutant.
After some tests i discovered that when i enabled the bypass-finals library, the mutation test didn't worked properly as you can see on the minimal project repo

Question Answer
Infection version 0.16.3
Test Framework version Codeception 4.0.3
PHP version 7.4
Platform Ubuntu
Github Repo - https://github.com/vitormelon/infection-test
Bypass-Finals - https://github.com/dg/bypass-finals

To reproduce this issue you only need to execute the infection on the github repo with the minimal project

If you want to see the project working without the bypass-final, and see de mutants been killed just comment the test: "testGivenMockedTheThingAndAnyInputThenShoudReturnTrue" and the "_before" method on the MyTest.php

I'm not sure if this is a problem that should be fixed on this repo, or on Bypass-Finals repo, but i think it's important to point because this issue took a lot of time to be uncovered

Log with the Bypass-Finals
You are running Infection with Xdebug enabled.

   ____      ____          __  _
  /  _/___  / __/__  _____/ /_(_)___  ____
  / // __ \/ /_/ _ \/ ___/ __/ / __ \/ __ \
_/ // / / / __/  __/ /__/ /_/ / /_/ / / / /
/___/_/ /_/_/  \___/\___/\__/_/\____/_/ /_/

Infection - PHP Mutation Testing Framework 0.16.3@52d597af80429d52dd6218fcb9766fc7653ec88c

Running initial test suite...

codeception version: 4.0.3

  22 [============================]  1 sec

Generate mutants...

Processing source code files: 2/2
.: killed, M: escaped, S: uncovered, E: fatal error, T: timed out

MMMMMMM                                              (7 / 7)

7 mutations were generated:
      0 mutants were killed
      0 mutants were not covered by tests
      7 covered mutants were not detected
      0 errors were encountered
      0 time outs were encountered

Metrics:
        Mutation Score Indicator (MSI): 0%
        Mutation Code Coverage: 100%
        Covered Code MSI: 0%

Please note that some mutants will inevitably be harmless (i.e. false positives).

Time: 1s. Memory: 14.00MB

Log without the Bypass-Finals
You are running Infection with Xdebug enabled.

   ____      ____          __  _
  /  _/___  / __/__  _____/ /_(_)___  ____
  / // __ \/ /_/ _ \/ ___/ __/ / __ \/ __ \
_/ // / / / __/  __/ /__/ /_/ / /_/ / / / /
/___/_/ /_/_/  \___/\___/\__/_/\____/_/ /_/

Infection - PHP Mutation Testing Framework 0.16.3@52d597af80429d52dd6218fcb9766fc7653ec88c

Running initial test suite...

codeception version: 4.0.3

  25 [============================]  1 sec

Generate mutants...

Processing source code files: 2/2
.: killed, M: escaped, S: uncovered, E: fatal error, T: timed out

.......                                              (7 / 7)

7 mutations were generated:
      7 mutants were killed
      0 mutants were not covered by tests
      0 covered mutants were not detected
      0 errors were encountered
      0 time outs were encountered

Metrics:
        Mutation Score Indicator (MSI): 100%
        Mutation Code Coverage: 100%
        Covered Code MSI: 100%

Please note that some mutants will inevitably be harmless (i.e. false positives).

Time: 0s. Memory: 14.00MB
@taigfs
Copy link

taigfs commented Jun 19, 2020

Fantastic issue, I had this problem as well. Thanks for the excellent clarification.

@BackEndTea
Copy link
Member

I think the bypass final also hijacks the file streamwrapper. I think the least we can do is error out if the streanwrapper is never used during the process

@vitormelon
Copy link
Author

I think the bypass final also hijacks the file streamwrapper. I think the least we can do is error out if the streanwrapper is never used during the process

That makes a lot of sense

@maks-rafalko
Copy link
Member

maks-rafalko commented Jun 21, 2020

I think the least we can do is error out if the streamwrapper is never used during the process

Unfortunately, I can't find a way on how to do it.

During mutation process, we create our custom autoload.php with Interceptor included, that disables original file stream wrapper and registers our own.

But in the provided example repo, BypassFinals::enable() is executed after our stream wrapper, right before the test execution in _before() method (setUp() alternative in Codeception).

So it's too late for Infection to check something.

Also, I didn't find any solution on how to understand what are the stream wrappers already registered (particular classes). The only thing we can check is whether the stream has a wrapper or not: stream_get_wrappers(), without any information about the classes.

Another idea is to add dg/bypass-finals to composer.json's conflict section so that users can't even install Infection if dg/bypass-finals is used, with a clear explanation in the Docs (yeah, we will still have to deal with PHAR distribution, but that's doable).

I will continue investigating how and what we can do it, but any help is much appreciated here.


UPD: we can try to do it with register_shutdown_function function, in which we can probably check the state of Interceptor (whether it was executed or not).


UPD2: related to dg/bypass-finals#9

@maks-rafalko
Copy link
Member

maks-rafalko commented Jun 21, 2020

Made a draft PR with register_shutdown_function: infection/include-interceptor#13

Before:

.: killed, M: escaped, U: uncovered, E: fatal error, T: timed out

.....                                                (5 / 5)

After:

Processing source code files: 2/2
.: killed, M: escaped, U: uncovered, E: fatal error, T: timed out

EEEEE                                                (5 / 5)

And part of the log file:

3) /infection/tests/e2e/Stream_Wrapper_Execution/src/FinalClass.php:11    [M] OneZeroInteger

--- Original
+++ New
@@ @@
 {
     public function get() : int
     {
-        return 1;
+        return 0;
     }
 }

  PHPUnit 8.5.7 by Sebastian Bergmann and contributors.

  ..                                                                  2 / 2 (100%)

  Time: 53 ms, Memory: 6.00 MB

  OK (2 tests, 3 assertions)

  Fatal error: Uncaught LogicException: Infection's IncludeInterceptor was not executed. Make sure you don't use any `file://` Stream Wrappers (like dg/bypass-finals) in /infection/include-interceptor/src/IncludeInterceptor.php:90
  Stack trace:
  #0 [internal function]: Infection\StreamWrapper\IncludeInterceptor::Infection\StreamWrapper\{closure}()
  #1 {main}
    thrown in /infection/include-interceptor/src/IncludeInterceptor.php on line 90

(note the fatal error at the end of the log)

I think we have 2 points here:

While the first issue is under our control and we can fix it, the second one is nice to have and optional. Anyway, even if we add some code to bypass-finals lib to decorate Infection's stream wrapper instead of replace it, there will be any other lib that can unregister Infection's stream wrapper as well.

Also, here is the e2e test that fails for the current master but will be fixed once we merge infection/include-interceptor#13 and update infection/include-interceptor dependency

@dkarlovi
Copy link

dkarlovi commented Sep 2, 2022

Whoever lands here, note that at this time dg/bypass-finals latest release is from 2020, but master is much more recent. This issue is fixed in master, but not yet released.

@maks-rafalko
Copy link
Member

Almost fixed*, but we are close.

see my comment dg/bypass-finals#9 (comment)

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

Successfully merging a pull request may close this issue.

6 participants