Check the test framework adapter version before proceeding #1227
Conversation
|
This looks like a good idea on the one hand, but on the other hand I don't see a substantial evidence backing the need for such drastic measures. Yes, there are couple of places where we have to track for older formats of coverage, but they can be (if not) thoroughly unit-tested, therefore there isn't a parsing problem per se. |
|
@sanmai it's not only about the coverage reports (so far even supporting PHPUnit 5.x is still ok), but also the test framework instrumentation which includes:
I'll be honest here: I'm not quite sure we are thorough in testing all of those either so I would not be surprised if we have a few non-working cases.
What minimum version we want to support means what minimum version we are willing to look at in case of a bug. So it also depends on your, personally I never used neither phpspec or codeception so I'm not keen to look at it even on recent versions at all. But I admit if that was only based on that, I would be a lot more aggressive in the minimum version too. So maybe a solution is to go first for a permissive arbitrary version and we can always adapt it later, e.g: After all, we don't want to break stuff for users. So one thing is say we are in theory pushing for phpspec 4.0.0 minimum and users come at us saying they would still like to have it work with phpspec 3.0.0, then we can lower that option and we know at least what minimum version to look at for our testing.
I would also like at some point us to be more through with the tests (see the full list above) and have this unit/integration tests at least for each major version, (e.g. if we support PHPUnit 5.x, then it means 5.x, 6.x, 7.x, 8.x, 9.x). Is that a lot? For sure, but also realistically between each major version something can break there. @maks-rafalko provided we go for a minimum version for each major framework (and if you disagree on the specific versions proposed up there I propose 1.0.0 for them...), would you agree to add this to |
sure, thanks, proposed versions sounds good to me
When I had the first implementation of codeception adapter, the minimum version with my fixes inside |
|
This also fixes #197 |
theofidry
changed the title
| @@ -127,11 +131,32 @@ public function getVersion(): string | |||
| return $this->version ?? $this->version = $this->retrieveVersion(); | |||
| } | |||
|
|
|||
| /** | |||
| * @throws UnsupportedTestFrameworkVersion | |||
| * TODO: add this to the interface upstream | |||
There was a problem hiding this comment.
is this TODO still relevant?
| ); | ||
| if ($matched === 0) { | ||
| throw new InvalidVersion(sprintf( | ||
| 'Expected "%s" to be contain a valid SemVer (sub)string value.', |
There was a problem hiding this comment.
probably just to contain? Just see such construction for the first time :)
* we can't release new versions of `infection/abstract-testframework-adapter` since no test framework implemented `checkVersion()`, see infection/infection#1227 * this will be easier transition for test framework to be updated
When instantiating an adapter, we are now checking the version used to warn the user of potential undesired behaviour if the version is not the expected one.
We are not interrupting the process to provide greater flexibility, but at the very least we will warn the user.
Outdated
I think it would be beneficial to provide an API for the test framework adapter to check that the test framework used is a valid version. Here is the example with this PoC with PHPUnit 5:
(verbose mode with the trace cut off)