Added support for Codeception test framework #143

tobiasstadler · 2018-01-30T19:51:21Z

Fixes #45

DavertMik · 2018-02-03T09:24:56Z

Any updates?

theofidry · 2018-02-03T09:33:18Z

Will try to review that in the course of next week, I'm sure the rest of the team will too. Thanks for the hard work and sorry to be a bit slow on this one :)

maks-rafalko

Thank you for your PR

Could you please rebase?
Could you please add an e2e test (see an example in tests/Fixtures/e2e) to prove it works with Codeception?

maks-rafalko · 2018-02-03T21:46:45Z

composer.json

@@ -46,7 +46,8 @@
    },
    "require-dev": {
        "phpunit/phpunit": "^6.1",
-        "mockery/mockery": "^1.0"
+        "mockery/mockery": "^1.0",
+        "codeception/codeception": "^2.3.8"


could you please explain why is it needed?

maks-rafalko · 2018-02-03T21:47:45Z

phpstan.neon

+        - '#Call to an undefined method PhpParser\\Node(.*?)#'
+        - '#Call to an undefined method Mockery\\MockInterface(.*?)#'


could you please explain why is it needed?

phpstan complains about a call to an undefined method Mockery\MockInterface::getMutation() in tests/TestFramework/Codeception/CommandLine/ArgumentsAndOptionsBuilderTest.php

maks-rafalko · 2018-02-03T21:59:27Z

src/TestFramework/AbstractTestFrameworkAdapter.php

@@ -74,12 +74,12 @@ public function __construct(
     *
     * @return string
     */
-    public function getExecutableCommandLine(string $configPath, string $extraOptions, bool $includePhpArgs = true): string
+    public function getExecutableCommandLine(string $configPath, string $extraOptions, bool $includePhpArgs = true, Mutant $mutant = null): string


TBH I don't think adding Mutant parameter here is a good way. This is why we have 2 ConfigBuilder implementations: InitialConfigBuilder and MutationConfigBuilder - two classes for different cases rather than one class with if () {} else {}.

If you really need to build different options for Initial test run and for mutations - let's create two separate ArgumentAndOptionBuilders (initial/mutation) with different parameters.

But see below suggestions about avoiding changes in this interface

this still needs to be fixed

maks-rafalko · 2018-02-03T22:00:28Z

src/TestFramework/Codeception/CommandLine/ArgumentsAndOptionsBuilder.php

+        ];
+
+        if ($mutant !== null) {
+            $options[] = '-o "paths: output: ' . $this->tempDir . '/' . $mutant->getMutation()->getHash() . '"';


could you please explain what is it used for?

maks-rafalko · 2018-02-03T22:02:59Z

src/TestFramework/Codeception/Config/Builder/InitialConfigBuilder.php

+
+    public function build(): string
+    {
+        return $this->originalConfigPath;


Maybe it's better to create all needed for Infection parameters in a new, temp config like it is done for PHPUnit and PHPSpec rather than adding/overriding config in ArgumentsAndOptionsBuilder?

This will probably help to avoid passing Mutant $mutant parameter to ArgumentsAndOptionsBuilder

In other words, I suggest to move this code:

$options[] = '-o "paths: output: ' . $this->tempDir . '"'; $options[] = '-o "coverage: enabled: true"'; $options[] = '--coverage-phpunit ' . CodeCoverageData::CODECEPTION_COVERAGE_DIR;

from ArgumentsAndOptionsBuilder here and create temp config. It will be consistent with the other Test Frameworks and will help to avoid changing the interface

unfortunately Codeception does not handle absolute paths in its config files, but maybe I can get it working using relative paths.

I'd suggest creating Codeception's Extension which will transform the current config the way you need it.
Extension can be published as a standalone package

maks-rafalko · 2018-02-03T22:04:56Z

src/TestFramework/Codeception/Config/Builder/MutationConfigBuilder.php

+        $mutatedFilePath = $mutant->getMutatedFilePath();
+        $interceptorPath = dirname(__DIR__, 4) . '/StreamWrapper/IncludeInterceptor.php';
+
+        // TODO change to what it was (e.g. app/autoload - see simplehabits)


please do not copy todo comments

maks-rafalko · 2018-02-03T22:05:27Z

src/TestFramework/Codeception/Config/Builder/MutationConfigBuilder.php

+
+    public function build(Mutant $mutant): string
+    {
+        $_SERVER['CUSTOM_AUTOLOAD_FILE_PATH'] = $customAutoloadFilePath = sprintf(


could you please expand on why do you need it?

This is needed in order to autoload the mutated file. As far as I know Codeception does not support a custom bootstrap file (as e.g. PHPUnit)

please use putenv instead of global variables

@sidz actually putenv() and getenv() are not thread safe so might be better to do it that way. I would however add an INFECTION_ prefix or something to it

Infection runs frameworks via symfony console which knows how to work with envs. We already use it :P.

p.s. IMHO Don't like to touch global variables

I'm not sure to follow, I don't see how using the Console component helps to make it thread-safe here.

Maybe for reference: symfony/recipes#156

Also I don't like to touch global variables much, but that's exactly what put/getenv are doings and regardless of what we are using we should use a prefix here IMO

maks-rafalko · 2018-02-03T22:09:27Z

src/TestFramework/Codeception/CustomAutoloadFilePath.php

+
+namespace Infection\TestFramework\Codeception;
+
+use Codeception\Events;


it will not work since codeception/codeception is in require-dev section and all files from this section are absent in PHAR distribution

we need to find a way to not use Codeception dependency at all. Infection should not depend on any test framework.

could you please explain what is the goal of this file, we will try to find another way to achieve the same goal

I don't thin this is a problem, because the code path is only used when the test framework is codeception.

The problem is not really if this is going to work, but rather than each test framework supported must be instrumented as a blackbox with no access to their internal. There is several reasons for that:

It is easier to maintain on our side and things are less likely to break (the end-user API is usually more stable than the code API, public like internal)

As we are going to integrate Infection with PHP-Scoper to be able to provide an isolated PHAR, this kind of dependence won't work anymore easily

This could work if this file won't be a part of Infection. (Isn't that already so?) Instead you could tell Codeception to include a file from the outside Infection. E.g. there's an IncludeInterceptor which is added to the tested code at one point. I'm not entirely sure you could do what you want there, but there's an opportunity.

maks-rafalko · 2018-02-03T22:11:22Z

tests/TestFramework/Codeception/Adapter/CodeceptionAdapterTest.php

+        ];
+    }
+
+    protected function tearDown()


please extend Mockery\Adapter\Phpunit\MockeryTestCase file. No need to use this tearDown function anymore

sidz · 2018-02-05T18:47:16Z

src/TestFramework/Factory.php

        throw new \InvalidArgumentException(
            sprintf(
                'Invalid name of test framework. Available names are: %s',
-                implode(', ', [TestFrameworkTypes::PHPUNIT, TestFrameworkTypes::PHPSPEC])
+                implode(', ', [TestFrameworkTypes::PHPUNIT, TestFrameworkTypes::PHPSPEC, TestFrameworkTypes::CODECEPTION])


could you please use TestFrameworkTypes::TYPES constant here?

sidz · 2018-02-05T18:47:55Z

src/Command/InfectionCommand.php

@@ -69,7 +69,7 @@ protected function configure()
                'test-framework',
                null,
                InputOption::VALUE_REQUIRED,
-                'Name of the Test framework to use (phpunit, phpspec)',
+                'Name of the Test framework to use (phpunit, phpspec, codeception)',


could you please use TestFrameworkTypes::TYPES constant here as we do in the ConfigureCommand?

sidz · 2018-02-05T18:55:43Z

tests/Fixtures/e2e/Codeception_Basic/run_test.bash

@@ -0,0 +1,21 @@
+#!/usr/bin/env bash
+if [[ $PHPDBG = 1 ]]


@borNfreee told that he had an issue with this variant on his mac. Could you please change this line to the if [ "$PHPDBG" = "1" ] ?

I had an issue with if [[ $PHPDBG=1 ]] on my Mac. if [[ $PHPDBG = 1 ]] works for me.

@borNfreee Does it also work for you?

yes, it does not work, see #151 (comment)

BackEndTea

I left a few comments, but overall this looks pretty good

BackEndTea · 2018-02-08T12:30:17Z

tests/e2e_tests

@@ -19,7 +19,7 @@ do
        composer install > /dev/null 2>&1
    fi

-    if [ -f "run_tests.bash" ]
+    if [ -f "run_test.bash" ]


If you rebase on master please undo this change.

The intended filename is run_tests.bash, the then clause not running the correct file has been fixed on master.

BackEndTea · 2018-02-08T12:30:46Z

tests/Fixtures/e2e/Codeception_Basic/run_test.bash

@@ -0,0 +1,21 @@
+#!/usr/bin/env bash


Please rename the file to run_tests.bash

BackEndTea · 2018-02-08T12:38:33Z

tests/TestFramework/Codeception/Config/YamlConfigurationHelperTest.php

+use PHPUnit\Framework\TestCase;
+use Symfony\Component\Yaml\Yaml;
+
+function rp(string $path) : string


Could you move this function into the test class as a helper function

theofidry

Added a few notes on top of what the others already said.

Thanks again for the great work, it's a significant piece!

theofidry · 2018-02-11T18:21:05Z

src/TestFramework/Codeception/CommandLine/ArgumentsAndOptionsBuilder.php

+     */
+    private $tempDir;
+
+    public function __construct(string $tempDir)


$tmpDir rather than $tempDir; same for the other places

theofidry · 2018-02-11T18:22:27Z

src/TestFramework/Codeception/Config/Builder/MutationConfigBuilder.php

+
+    public function build(Mutant $mutant): string
+    {
+        $_SERVER['CUSTOM_AUTOLOAD_FILE_PATH'] = $customAutoloadFilePath = sprintf(


@sidz actually putenv() and getenv() are not thread safe so might be better to do it that way. I would however add an INFECTION_ prefix or something to it

theofidry · 2018-02-11T18:27:00Z

src/TestFramework/Codeception/CustomAutoloadFilePath.php

+
+namespace Infection\TestFramework\Codeception;
+
+use Codeception\Events;


The problem is not really if this is going to work, but rather than each test framework supported must be instrumented as a blackbox with no access to their internal. There is several reasons for that:

It is easier to maintain on our side and things are less likely to break (the end-user API is usually more stable than the code API, public like internal)

As we are going to integrate Infection with PHP-Scoper to be able to provide an isolated PHAR, this kind of dependence won't work anymore easily

theofidry · 2018-02-11T18:28:43Z

tests/Fixtures/e2e/Codeception_Basic/tests/_support/UnitTester.php

+ * @method void comment($description)
+ * @method \Codeception\Lib\Friend haveFriend($name, $actorClass = NULL)
+ *
+ * @SuppressWarnings(PHPMD)


is this @SuppressWarnings(PHPMD)necessary?

theofidry · 2018-02-11T18:29:24Z

tests/Fixtures/e2e/Codeception_Basic/tests/unit.suite.yml

+modules:
+    enabled:
+        - Asserts
+        - \Helper\Unit


missing line return

I don't think the leading \ is necessary is it?

theofidry · 2018-02-11T18:30:09Z

tests/TestFramework/Codeception/Adapter/CodeceptionAdapterTest.php

+    /**
+     * @dataProvider passProvider
+     */
+    public function test_it_determines_whether_tests_pass_or_not($output, $expectedResult)


(string $output, bool $expectedResult)

theofidry · 2018-02-11T18:30:45Z

tests/TestFramework/Codeception/Adapter/CodeceptionAdapterTest.php

+
+    public function passProvider()
+    {
+        return [


you can use yield instead:

yield ['OK, but incomplete, skipped, or risky tests!', true]; yield ['OK (5 tests, 3 assertions)', true];

theofidry · 2018-02-11T18:31:58Z

tests/TestFramework/Codeception/Config/Builder/InitialConfigBuilderTest.php

+    {
+        $this->filesystem = new Filesystem();
+
+        $this->workspace = sys_get_temp_dir() . '/infection-test' . \microtime(true) . \random_int(100, 999);


I think this is used elsewhere as well, if that's the case I would suggest to either have a helper for this or an abstract test case for those file-system related tests

theofidry · 2018-02-11T18:39:07Z

tests/TestFramework/Codeception/Config/YamlConfigurationHelperTest.php

+
+    public function test_it_can_build_config_with_custom_paths()
+    {
+        $originalContent = <<<EOS


'YAML' instead? I think IDEs would be able to pick the syntax highlighting

BackEndTea · 2018-02-23T20:53:02Z

Could you run composer cs:fix?

BackEndTea · 2018-02-23T21:01:19Z

I'll give this good look over after the weekend, and see if i can break anything.

Fenikkusu · 2018-03-20T20:02:40Z

src/TestFramework/Factory.php

+
+            return new CodeceptionAdapter(
+                new TestFrameworkExecutableFinder(CodeceptionAdapter::EXECUTABLE),
+                new CodeceptionInitialConfigBuilder($this->tempDir, $this->projectDir, $codeceptionConfigContent, $this->infectionConfig->getSourceDirs()),


I was just trying this out on a project. A recent MR has changed the variable $this->tempDir to $this->tmpDir. I had to fix this before I could use the PR.

…OLOAD_FILE_PATH

…s actuall yaml

Fenikkusu · 2018-03-22T13:54:24Z

src/TestFramework/Factory.php

+            $codeceptionConfigContent = file_get_contents($codeceptionConfigPath);
+
+            return new CodeceptionAdapter(
+                new TestFrameworkExecutableFinder(CodeceptionAdapter::EXECUTABLE),


Not Found For Me. Changed It To TestFrameworkFinder and started working again.

was renamend to TestFrameworkFinder in 955c11e

Fenikkusu · 2018-03-23T13:03:01Z

So far so good. I'm having issues running it through a docker image ( #254 ), but I think that is related more to infection than the codeception work.

sidz · 2018-03-23T13:09:25Z

@Fenikkusu first of all this PR has issues which should be fixed. @tobiasstadler could you please check the latest Travis build https://travis-ci.org/infection/infection/builds/357027746?utm_source=github_status&utm_medium=notification and fix all issues

Fenikkusu · 2018-03-30T14:15:32Z

I've come across an issue I didn't come across originally. If you are using params:env:.env, The system chokes because it can't find the .env file. I'm working on resolving the issue and have a partial fix at https://github.com/twistersfury/infection:codeception. However, this resulted in additional errors I have not yet had the time to resolve.

Fenikkusu · 2018-03-30T20:35:03Z

I've figured out the additional errors. I was running into two issues. The first was I had an acceptance suite installed, but no actual tests installed. When coverage ran, codeception choked because it was trying to connect to the acceptance url, which didn't exist. This error was easy enough for me to resolve, just by disabling the suite. However, it brings up the point of should the ability be added as part of the configuration to be able to specify which suites are ran?

The next issue took me a bit to track down. This time, phpunit was choking on tokenization for coverage with phpunit. Basically I had files in the project that were empty, but weren't PHP files. I tracked down the command infection was running and tried to replicate the specifics. I ultimately realized that when the Codeception suite runs, it is setting the configuration of coverage files to '*' based off the infection config. IE:

    "source": {
        "directories": [
            "src"
        ]
    }

Is translated into:

coverage:
  enabled: true
  include:
    - src/*

Because it is '', it is including all files, even non-php ones. I'm fairly certain if we just remove the '' that will fix the issue. My question though, at this point is if there is a specific reason why * was used instead of just using the folder path?

DavertMik · 2018-03-30T22:25:33Z

However, it brings up the point of should the ability be added as part of the configuration to be able to specify which suites are ran?

Yes, there is no sense to run mutation testing for acceptance or functional testing. So for those suites inflection should be disabled, that's for sure.

Fenikkusu · 2018-03-30T23:08:35Z

So for those suites inflection should be disabled, that's for sure.

I agree that they should be disabled, but I still think the ability needs to be passed onto the user via configuration. I say that because the suites are not limited to unit, functional, and acceptance. For example, I've a project that has a suite called 'legacy' and another called 'coverage'. Both of these suites run under the pretense of unit testing and should be ran as part of the testing. I think in the end, an option in the json configuration needs to be added for suites. When not specified, it defaults to 'unit'. When specified, it runs all the suites specified. This could easily be a string that just directly injects into the command line arguments.

Fenikkusu · 2018-03-30T23:28:01Z

Update: I tested the code using just the path and not the '*'. I still run into the same issue. The easiest solution is to just change it to be *.php. Done and done. The only problem I see with that is that the possibility of file names not ending in .php. IE: php5, php4, and phtml are all considered valid extensions, though rare (with phtml likely not being a class).

Fenikkusu · 2018-04-06T16:48:20Z

Noting Additional Issue: If codeception errors (IE: use db module without an accessible db), the error isn't getting properly echoed by infection.

Update: Interestingly it is actually working on my local comp. The CI however isn't outputting anything...

maks-rafalko · 2018-04-06T17:01:20Z

the error isn't getting properly echoed by infection

that sucks. Could you please try to set an error hanlder as it is done in #275 and see if it helps?

Fenikkusu · 2018-04-06T18:31:22Z

After a bit of digging, I was able to figure out how to get this to replicate consistently. If I'm not debugging, I never get any output from the error (again, use the db module with invalid db). If I put in a break point and force the codecept command to finish before the incept command has time to kick off the wait process, I get the proper output. AKA, break on line 303 of Symfony's Process.php file and step the codecept run to the next line. Let the codecept finish, then continue running infection. In this case, you will get the correct output.

Fenikkusu · 2018-04-07T00:50:27Z

that sucks. Could you please try to set an error hanlder as it is done in #275 and see if it helps?

@borNfreee, I tried that as well but saw no difference. At this point it appears to be a race condition in the underlying symfony framework.

BackEndTea · 2018-08-09T16:31:46Z

@tobiasstadler are you still working on this?

tobiasstadler · 2018-08-09T18:58:11Z

Sorry, I am not working on this anymore.

BackEndTea · 2018-08-09T19:13:30Z

Sorry, I am not working on this anymore.

I'm gonna go ahead and close this PR then. Thank you for the work you put into it.
Even though this won't be merged it may be usefull if someone wants to pick it up at a later point.

Fenikkusu · 2018-08-13T17:58:11Z

I've already done number of updates to this, though I haven't refreshed it in a couple of months. I'll open a new MR once I have a chance to rebase and clean up. I've been using it in a few applications, so I think it's about 80% there.

maks-rafalko · 2019-11-27T19:48:11Z

Implemented in #800

Added support for Codeception test framework

3139604

theofidry requested review from maks-rafalko, corpsee, theofidry and sidz and removed request for corpsee February 3, 2018 09:33

maks-rafalko requested changes Feb 3, 2018

View reviewed changes

tobiasstadler added 2 commits February 4, 2018 20:49

Merge remote-tracking branch 'upstream/master'

ab6f63b

Incorporated changes requested by @borNfreee

1947a90

sidz reviewed Feb 5, 2018

View reviewed changes

maks-rafalko mentioned this pull request Feb 5, 2018

Behat support #147

Closed

BackEndTea mentioned this pull request Feb 5, 2018

Allow multiple test frameworks to be used for one run #157

Open

Incorporated changes requested by @borNfreee and @sidz

e1418cf

BackEndTea reviewed Feb 8, 2018

View reviewed changes

theofidry reviewed Feb 11, 2018

View reviewed changes

tobiasstadler added 2 commits February 23, 2018 21:24

Inorporated changes requested by @BackEndTea and @theofidry

f3dd34d

Merge remote-tracking branch 'upstream/master'

5321032

Fenikkusu suggested changes Mar 20, 2018

View reviewed changes

tobiasstadler added 4 commits March 21, 2018 10:19

Merge remote-tracking branch 'upstream/master'

de9d263

Fixed code sytle violations using cs:fix

b956f27

Renamed CUSTOM_AUTOLOAD_FILE_PATH to INFECTION_CODECEPTION_CUSTOM_AUT…

269a7d3

…OLOAD_FILE_PATH

Use YAML instead of EOS as heredoc terminator if the heredoc string i…

3d6aea1

…s actuall yaml

Fenikkusu suggested changes Mar 22, 2018

View reviewed changes

TestFrameworkExecutableFinder

d9f8b92

was renamend to TestFrameworkFinder in 955c11e

tobiasstadler added 2 commits March 22, 2018 19:22

Added fixture files/directories

9e8e7e0

Merge branch 'master' into master

4fddaff

Fenikkusu mentioned this pull request Mar 28, 2018

Updating Dist Config File Locator #269

Merged

3 tasks

Fenikkusu mentioned this pull request Apr 5, 2018

Debugging #291

Closed

maks-rafalko mentioned this pull request Jun 8, 2018

Atoum unit testing framework integration #381

Closed

BackEndTea closed this Aug 9, 2018

maks-rafalko mentioned this pull request Oct 2, 2019

Add support for Codeception Test Framework #800

Merged

6 tasks

		- '#Call to an undefined method PhpParser\\Node(.*?)#'
		- '#Call to an undefined method Mockery\\MockInterface(.*?)#'


		namespace Infection\TestFramework\Codeception;

		use Codeception\Events;

Added support for Codeception test framework #143

Added support for Codeception test framework #143

Conversation

tobiasstadler commented Jan 30, 2018

DavertMik commented Feb 3, 2018

theofidry commented Feb 3, 2018

maks-rafalko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maks-rafalko Feb 3, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maks-rafalko Feb 3, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sidz Feb 8, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sanmai Apr 6, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BackEndTea left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

theofidry left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BackEndTea commented Feb 23, 2018

BackEndTea commented Feb 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Fenikkusu commented Mar 23, 2018

sidz commented Mar 23, 2018 • edited

Fenikkusu commented Mar 30, 2018

Fenikkusu commented Mar 30, 2018

DavertMik commented Mar 30, 2018

Fenikkusu commented Mar 30, 2018

Fenikkusu commented Mar 30, 2018

Fenikkusu commented Apr 6, 2018 • edited

maks-rafalko commented Apr 6, 2018 • edited

Fenikkusu commented Apr 6, 2018

Fenikkusu commented Apr 7, 2018

BackEndTea commented Aug 9, 2018

tobiasstadler commented Aug 9, 2018

BackEndTea commented Aug 9, 2018

Fenikkusu commented Aug 13, 2018

maks-rafalko commented Nov 27, 2019

maks-rafalko Feb 3, 2018 •

edited

maks-rafalko Feb 3, 2018 •

edited

sidz Feb 8, 2018 •

edited

sanmai Apr 6, 2018 •

edited

sidz commented Mar 23, 2018 •

edited

Fenikkusu commented Apr 6, 2018 •

edited

maks-rafalko commented Apr 6, 2018 •

edited