Added more configuration to travis.yml for matrix testing

[DonCallisto]

@alexislefebvre @tarlepp let's continue #112 here.

What do you guys think? Is this good? Are we ready for version 2?

[DonCallisto]

Another possible configuration is to keep here php version that I've dropped and even phpunit ones and drop only behat 2 that's no longer maintained. After this, build a job matrix where we can use right requirements for each configuration. This will let users stay with an older php version (even 7) and still upgrade to symfony 3 if they're on symfony 2. I will try to make an additional commit to compare them.

[DonCallisto]

Please guys, take a look to single commits: in the first I've dropped a lot of things in composer.json, in the second I've tried to keep the best combination possible as supporting that wide range of symfony and php version is not so easy.

In the second one, for instance, we claim to support at least php 5.6 and symfony 4 components but that's impossible: of course if someone try to use this package with symfony 4 will be stopped he hasn't correct version of php. Only thing I'm worried about is phpunit that can be installed in the older version along with php 7.2 but, maybe, that's not our problem. So I'm for keeping only the second commit (maybe shortening travis.yml if anyone knows how and if it's possibile to shorten it). I still doubtful about release version: bug fix, minor or major? 🤔

Which commit do you prefer? Which version would you release? Please, remember that behat 2 is still dropped and it's the only common "drop" here.

Is also worth notice that in the second commit I didn't found a method to shorten the build matrix: if anyone knows how to improve it, please let me know.

[francoispluchino]

@DonCallisto To respond to your solicitation in issue #117, Symfony 4.x requires PHP ^7.1.3, alors que Symfony 3.x requires PHP ^5.5.9|>=7.0.8 and Symfony 2.x requires PHP >=5.3.9. I will opt for a more understandable version of each Symfony major version test with each version of PHP in the matrix.

On this subject, the importance of Symfony version should not really be a topic, because it is strongly recommended, and Symfony 4 Flex indicates it well with each installation, that Phpunit should not be in the dependencies of the project, even in dev. So, it is recommended to install Phpunit globally. As a result you can very well reduce version compatibility of Symfony.

This is only a point of view, so do not take it into account if you do not agree ;-).

[DonCallisto]

@francoispluchino looking at last build it seems that #117 break something

[francoispluchino]

It is a problem with the cast of env variables (string vs int or float).

[DonCallisto]

Yes I saw it but don't know how to fix

[DonCallisto]

I suppose that symfony/process 2.7.0 is the cause

[DonCallisto]

@francoispluchino bumping from 2.7 to 2.8 don't produce any result, but somehow lower deps are breaking this

[DonCallisto]

As I was wondering with my last commit - that was just for a quick test, of course - it seems that there's no easy way to test this for all symfony versions in the way we are doing it right now: is this a smell of what? Do we need to write a more robust test somehow?

Only solution could be test only for presence of $_SERVER keys regardless of values and test for fastest env variables directly with values.

At the moment I can't think to a better solution /cc @francoispluchino

[francoispluchino]

I will look at this problem tomorrow morning.

[DonCallisto]

@francoispluchino sometimes tests fails after some modification (see last commit that should've fixed tests); I suppose that #117 break something. Could you also try to take a look?

[francoispluchino]

The PR #117 does not bring of BC, it is simply a problem of comparison on the whole array with a component which does not have the same behavior between the different versions, and that tests use this component. Before, the array contained only the values of Fastest, so the error was not visible. The values with an error are those of Phpunit, but I have not found the problem yet.

As I said above, we can test only the keys, not the values, or just convert the values of the two arrays to String for the comparison.

[DonCallisto]

That's not true as tests seems to fail randomly in my host, so it's not a problem of version I suppose. Moreover we expect some fixed values like ENV_TEST_CHANNEL that we provide in input, but sometimes it changes as it seems that $_SERVER replace variables (in a random fashion) that our EnvCommandCreator creates. Please check it out yourself.

[francoispluchino]

I did not say that the tests fail randomly, but that the tests fail according to a used version of a component. I finish a work, and I bring you a solution right after.

[DonCallisto]

I did not say that the tests fail randomly

No, you didn't but I did as I've tried locally :)

[DonCallisto]

@francoispluchino please try yourself to test (locally) the branch as it is and you'll notice what I'm saying here. Of course if you test only keys you'll make all tests green

[francoispluchino]

@DonCallisto You can see my fix in the PR #122.

[DonCallisto]

I'm still not sure about it. Take a look to this build, especially at this

1) Liuggio\Fastest\Process\ProcessFactoryTest::shouldCreateACommandUsingParallelTests
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
-    'ENV_TEST_ARGUMENT' => 'tests/Process/ProcessFactoryTest.php'
-    'ENV_TEST_CHANNEL_READABLE' => 'test_1'
-    'ENV_TEST_INC_NUMBER' => '4'
-    'ENV_TEST_CHANNEL' => '1'
-    'ENV_TEST_CHANNELS_NUMBER' => '2'
-    'ENV_TEST_IS_FIRST_ON_CHANNEL' => '0'
+    'ENV_TEST_ARGUMENT' => 'fileA'
+    'ENV_TEST_CHANNEL_READABLE' => 'test_2'
+    'ENV_TEST_INC_NUMBER' => 10
+    'ENV_TEST_CHANNEL' => 2
+    'ENV_TEST_CHANNELS_NUMBER' => 10
+    'ENV_TEST_IS_FIRST_ON_CHANNEL' => 1
 )

As you can see it's not a matter of cast: values are really different each others and that's the "random failure" a was talking about: sometimes it fails this way but I don't know why. I'm pretty sure your PR won't fix this

[francoispluchino]

Ok for your use case. This does not happen when you use directly Phpunit, but there is a conflict using Fastest to test Fastest. My PR fixes all the problems except this one.

[francoispluchino]

I understood the problem. By using Fastest to test Fastest, and since Fastest now inject the environment variables into the test process, the sub-process of test retrieves the environment variables including the all ENV_TEST_ * variables, and that $_SERVER + $_ENV + [] is different of array_merge($_SERVER, $_ENV, []) (the first does not override the values), we get a conflict over the environment variables of Fastest. However, even using array_merge, not all variables are overwritten like ENV_TEST_INC_NUMBER for example (because injected elsewhere).

So to fix this problem, we must make sure that the variables do not come into conflict by deleting them if they already exist.

You can see my updated PR.

[alexislefebvre]

The fact that variables are passed to sub-processes remind me of a problem I encountered recently, I had to use the @preserveGlobalState disabled annotation to avoid putting data in the environment of the sub classes.

Hope this helps.

You can see this commit for reference: https://github.com/liip/LiipFunctionalTestBundle/pull/351/commits/99af5044c27308b38c822a5acced4071a75004ce#diff-94415f7053caa5669bb4007b33b79bcb

See also the PHPUnit's documentation: See related documentation: https://phpunit.de/manual/current/en/appendixes.annotations.html#appendixes.annotations.preserveGlobalState

[francoispluchino]

@alexislefebvre Thank you for your info, but the problem is not the non-serialisable variables, but only the conflict with the variables of Fastest. Delete all variables of Fastest works fine.

[DonCallisto]

Resolved in #122