Work is based on the following resources
This is the RFC with detailed description of Package Design Principles applied to Infection, that show the benefits of splitting infection/infection to separate packages. The main topic here is Test Framework Adapters, but splitting Infection is not limited to them (see another RFC for extracting Mutators).
For me, there are no doubts we will split it. The question is how deep we will go.
Principles about package cohesion
Common Reuse Principle
Classes that are used together are packaged together
We clearly violate this priciple. People who use only PHPSpec adapter should not download Codeception/PHPUnit adapters.
At the moment, infection/infection contains classes that are not all used by everyone who uses Infection package in their project, so it violates the Common reuse principle. It also contains classes (the same classes actually) that are not closed against the same kinds of changes, so the package violates the Common closure principle. More on it later.
Dependencies of Test Framework
Another thing that shows we violate this package principle is that infection/infection requires symfony/yaml. The key thing is that Infection has nothing to do with YAML. It's a dependency of Codeception and PHPSpec adapters, but not Infection itself. Each time you use only PHPUnit adapter (and don't use Codeception/PHPSpec), you have
- the code you don't really need, that is not executed
- one more reason to be affected by additional dependency
- one more possible conflicting dependency when install Infection via Composer; and so on
Cohesion
When we extract each Test Framework Adapter to a separate package, they will be coherent: all the classes a separate package contains are about the same thing. When user needs PHPUnit adapter, all the classes from infection/phpunit-adapter will be used together. And no classes will be used and downloaded from other adapters.
The Common closure principle
- The classes in a package should be closed together against the same kinds of changes. A change that affects a package affects all the classes in that package.
- The primary justification for this principle is that we want a change to be limited to the smallest number of packages possible.
When a new version of the package becomes available, the user will upgrade their project (composer update infection/infection) to require the new dependency version. But they only want to do so if the changes we made to the package have something to do with the way the package is used in their project, since every upgrade requires them to verify that the code still works correctly with the new version of our package.
What does it mean to us? When we, for example, fix a bug for the code related to Codeception adapter, people who use only PHPSpec adapter should not be affected. They should not download the new version of infection/infection (this is how it works currently with the monolith), which may or may not have unwanted side-effects or just bring many unrelated changes from the repository. Instead, only infection/codeception-adapter would be updated.
As a package maintainer you should follow the Common closure principle to prevent yourself from “opening” a package for all kinds of unrelated reasons. It helps you prevent bringing out new releases that are irrelevant to most of your users. With this goal in mind the principle advises you to put classes in different packages if they have different reasons to change.
Page 153 - Principles of Package Design
Another positive benefits:
- we will be able to set version constraints for any dependency. See this my commit https://github.com/infection/infection/commit/73bb17a0f98d6fda67a276d29648e5a54170767d#diff-d9b88806f4deaae39c43c22fb7f31d0fR214-R228: it checks that
codeception/codeception must be of ^3.1.1 version. This should not be done in the main repository, but only in adapter
- we will be able to track packages statistics separately:
- how many downloads
infection/phpunit-adapter has, the growing (or not) installs diagram
- is it popular or not (stars, dependents), etc.
- we will be able to release test framework adapters independent of
infection/infection. This is a big win since people who use PHPUnit only will not be affected by a bugfix (or new introduced bug, who knows) in the recently updated PHPSpec adapter's code
In this section about cohesion in Package Principles, I conclude that we need to extract the following 3 separate packages from infection/infection:
infection/phpunit-adapterinfection/phpspec-adapterinfection/codeception-adapter
Principles about packages coupling
Imaging we already extracted Test Framework Adapters to separate packages. Now, let's think what for example infection/phpunit-adapter will depend on. Here we have a couple of ways to go:
infection/phpunit-adapter depends on infection/infection and uses TestFrameworkAdapter interface located in the main infection/infection core packageinfection/phpunit-adapter depends on a small, abstract and highly stable package, e.g. infection/abstract-testframework-adapter wich contains only abstractions and probably DTOs for Test Framework Adapters (or infection/contracts, read below)
The following coupling principles will help us determining which of the ways above is more suitable for Infection.
The Stable Dependencies Principle (SDP)
Let's start from definitions.
- Stable package has no or few dependencies it depend on
- Unstable package needs to be updated each time its dependencies upgraded
- Responsible package has many dependents - packages that depend on this package
- Irresponsible package is not used by any packages
Packages that are more independent and responsible should be considered highly stable. Those are packages that don’t need to change because of a change in one of their dependencies, but they also can’t easily change themselves because other packages heavily depend on them.
And here is the Stable Dependencies Principle:
The dependencies between packages in a design should be in the direction of the stability of the packages. A package should only depend upon packages that are more stable than it is.
Actually, we can calculate the "instability value" and see how our packages can be designed and how this plays with this package design principle.
Instability value
There is a formula to measure Instability of any package.
I = Cout / (Cin + Cout)
where
- I - instability
- Cout - the number of classes outside the package that any class inside the package depends upon
- Cin - the number of classes outside the package that depend on a class inside the package.
I value will be between 0 and 1 where 1 indicates that the package is maximally unstable and 0 indicates that it is maximally stable.
A highly stable package is responsible: it has many dependents, so Cin is a high number. A highly unstable package is very dependent. It has many dependencies, so Cout is a high number.
How can our packages be designed? There are at least 2 possible ways.
1. TestFrameworkInterface is inside infection/infection
The first case, when the TestFrameworkInterface is placed inside infection/infection, and our Test Framework Adapters packages depend on infection/infection.
From the first look, this is a simple and working solution. But there is a big issue: if the infection/codeception-adapter would depend on infection/infection to get the TestFrameworkAdapter interface from there, we would need to update its dependencies each time infection/infection is changed, even when the change is absolutely unrelated to the adapter's behaviour.
This is how the infection/codeception-adapter's composer.json would look like:
{
"require": {
"symfony/yaml": "^3.4.29 || ^4.0 || ^5.0",
"infection/infection": "^0.13 | ^0.14 | ^0.15 ... and so on"
}
}
Each time a new infection/infection version is released (even with the changes irrelevant to infection/codeception-adapter), we will need to update composer.json file to make this adapter compatible with the new version. And each such update means we need to test it to avoid potentially broken integration between adapter and the core Infection. We don't know, whether the new release of infection/infection affects us or not, but we still need to adopt these changes, and composer outdated will really show that adapter's dependency is not up to date.
We will end up with the similar PRs as this one, constantly updating infection/infection's versions.
What about Instability value for this way of package relations?
I = Cout / (Cin + Cout)
Note: 14 is a number of packages infection/infection is currently depends on.
This way of packages design is clearly violates the Stable Dependencies Principle, because infection/codeception-adapter with I=0.5 depends on a package infection/infection with a bigger I=0.875. But according to the Stable Dependencies Principle, the dependencies between packages should be “in the direction of the stability of the packages" (or "in the direction of decreasing Instability"). Hence a red line.
2. TestFrameworkAdapter is inside highly stable infection/abstract-testframework-adapter
So, we need to make infection/codeception-adapter (and other adapters) to depend on something more stable, more reliable. Here is what we can do:
infection/abstract-testframework-adapter is a highly stable because it depends on 0 packages. It has only 1 reason to change - changing the Public API of TestFrameworkAdapter.
infection/codeception-adapter's composer.json in this case would be like this:
{
"require": {
"symfony/yaml": "^3.4.29 || ^4.0 || ^5.0",
"infection/abstract-testframework-adapter": "^1.0.0"
}
}
Now, when the new version of infection/infection is released, our adapters are not affected. The really one reason to update adapter's dependency is when we change our abstraction, which is a rare thing.
Here is how Instability values look like:
In this example each package depends on more stable packages, which is what this principle tells us to do.
There is a similar example that Mattias describes in his book about gaufrette/filesystem package:
[...] After we moved the Gaufrette\Filesystem class and the Gaufrette\Adapter interface out of the main package and into a small and very stable package, it became a lot safer to use that class and/or interface in another package because the freshly created knplabs/gaufrette-filesystem-abstraction package is very stable.
[...] The lesson to be learnt here is that when you have the ability to split a package into multiple packages with different levels of stability, you should do it. You will make it easier for people to use your code in their projects, without introducing any more instability.
Page 216 - Principles of Package Design
The Stable Abstractions Principle (SAP)
Packages should depend in the direction of abstractness.
Packages that are maximally stable should be maximally abstract. Instable packages should be concrete. The abstraction of a package should be in proportion to its stability.
This principle again makes sure that the more abstract a package we depend on, the less chances it will be changed. Because usually changes are done in concrete classes/packages, but not abstract. Abstract packages supposed to be stable. Over a longer period of time it will stay the same.
As in the previous principle, we can calculate the value of Abstractness.
A = Cabstract / (Cconcrete + Cabstract)
I will show the values only for the second case, when we have a infection/abstract-testframework-adapter:
As you can see, infection/abstract-testframework-adapter is very stable (I=0) and highly abstract (A=1). This means that this package conforms to both Stable Dependencies and Stable Abstractions principles and is a fundamental block of the package hierarchy.
By the way, it's ok adapters be non abstract (and quite instable), because such packages are real concrete implementation of some abstraction. And there is no doubt they will change during the time.
What happens if an interface TestFrameworkAdapter is part of a highly unstable package infection/infection? The answer is:
[...] Then it is consequently not safe to depend on that package. The unstable package is likely to change. The interface inherits the instability of its containing package.
Page 221 - Principles of Package Design
That's it.
The Acyclic Dependencies Principle
The dependency structure between packages must be a directed acyclic graph, that is, there must be no cycles in the dependency structure.
We don't have another packages yet so we are not affected by cycles between packages ;)
Misc
infection/contracts repository
Now, we see that we need to extract Test Framework Abstraction to a separate repository. But this is not the last one. We will have at least one more abstraction - MutatorInterface. So it will probably be convenient to not have 2 separate repositories for each abstraction, but one infection/constract. Anyway, I'm not suggesting to create it right now, let's start with infection/abstract-testframework-adapter and then we will see...
PHAR prefixing and bundling all adapters together
You can ask how are we goin to prepare our PHAR file after splitting Infection to different repositories and extracting Test Framework Adapters. The answer is very simple - we will always bundle Codeception, PHPUnit, PHPSpec repositories inside PHAR file. PHAR users even won't notice this splitting.
Autodiscovering of Packages
For those who uses Infection as a composer dependency.
What is really interesting, is how we can install different "plugins" for Infection, be it Test Framework Plugin or other future plugins like custom Mutators. There is an approach of autodiscovering via composer packages, for example:
- https://psalm.dev/docs/running_psalm/plugins/authoring_plugins/#authoring-composer-based-plugins
- https://github.com/phpstan/extension-installer
The idea is very simple. A package has a special type inside composer.json file, for example "type": "infection-plugin".
{
"name": "infection/codeception-testframework-adapter",
"type": "infection-plugin",
"require": {
"php": "^7.2.9",
"ext-dom": "*",
"ext-libxml": "*",
"symfony/yaml": "^3.4.29 || ^4.0 || ^5.0"
},
"conflict": {
"codeception/codeception": "<3.1.1" // isn't it cool?
},
}
Then, Infection analyzes composer.lock file and autoregister all Infection plugins. The end user will just do the following change in the project's composer.json:
"infection/infection": "^0.15",
+ "infection/codeception-testframework-adapter": "^1.0.0"
and it's done - Infection automatically finds a registers adapter.
Of course, we will be able to add plugins in infection.json file as well. But autodiscovery is a cool feature.
Last words
We don't need to treat our abstract packages with extension points immediately final and stable for users from the very first release. No. This is just the first step to be able to use flexible and useful Packages. And only after we quite stabilize them, we can mark them public (it's not the same as 1.0.0) and generally available. Until then - these packages can be "private".
At this point, I hope nobody have any doubts that we have to split Infection.
Feature Request RFC
10.3k Jan 1, 2023
147 Jun 20, 2022
1.4k Nov 29, 2022
4.6k Jan 7, 2023
18.8k Jan 4, 2023
67 Sep 12, 2022
5.9k Dec 27, 2022
363 Dec 19, 2022
894 Jan 4, 2023
219 Dec 17, 2022
236 Nov 28, 2022
24 Apr 22, 2022
2k Dec 31, 2022
13 Jun 12, 2022
10 Dec 31, 2022
400 Jan 5, 2023
16 Oct 6, 2022