Force Mutators to include remedies #1907
Conversation
In order to improve the developer experience this PR forces Mutators to include remedies in their definitions.
There was a problem hiding this comment.
For the infection team that was not part of the Discord conversation: the goal is to document the remedies which can later be used to point the user how to address some escaped mutations. To avoid pushing all remedies at once out of fear to end up with "low quality ones just for the sake of having it", this PR can introduce the requirement, and the remedies be fixed over time in separate PRs.
|
|
||
| public function mutatorsProvider(): iterable | ||
| { | ||
| $mutatorFactory = SingletonContainer::getContainer()->getMutatorFactory(); |
There was a problem hiding this comment.
The reason for this inline call to the container is because providers run before setUp methods, in which case a call to something like $this->mutatorFactory would be null.
|
@infection/core are you fine with the idea? If so I would rebase it |
|
If the idea is to ultimately have all the remedies, then absolutely yes. Just today I was discussing with @Wirone this ticket (in context of his question about not always understanding how to kill mutants). Is it a coinsidence? :) Also, as a followup for this ticket: we can add links (or inline explanations) on how to kill mutant to our GitHub annotations, so contributor will better understand what to fix when mutant is escaped. I like how Psalm adds short links to the output with found issues, and you can open it and read explanations. This is something I would like to have for Infection too (probably integrated with Playground to show "killing tests" there). |
No, it's not a coincidence, after our conversation I asked @ttomdewit what's the state of this, it seems like it sparked some reaction 🙂. |
It did indeed! @theofidry and I chatted a bit in DM over Discord! Let me know if I need to do anything to get this in, and probably no sooner than June I will look into documenting mutants one PR at a time (no promises at the moment, not doing very well mentally). |
From seeing fast evolving docs from Rector, that did that approach, I have mixed feelings about it. Ideally, this would be done with the links bound to the released version, and the doc is versioned too. For example if you had the following code: <<<REMEMDY
Short explanation.
See more at https://docs.infection.com/remedies/<mutant-name>
REMEDY;When building infection, you would have the shipped code updated to point to a specific version: <<<REMEMDY
Short explanation.
See more at https://docs.infection.com/v0.18/remedies/<mutant-name>
REMEDY;And when accessing the docs you could easily jump from a version to another. Also note that the version the code is bound to would be the matching minor version to allow us to update the docs still. But I think this would be quite a lot of work... So meanwhile I guess even if the remedy description is a bit lengthy it is fine. Or we have to live with the fact that it points to the latest updated doc which will be updated hence if using an old version of infection it may have dead links. |
This PR is a rebased version of #1907. It currently skips all but one mutator to set a baseline. Existing mutators should no longer be skipped in the future and this check ensures that new mutators will require a remedy to be documented. There is more to do with remedies to, for example some ideas of #1907 (comment), but I think it is vastly out of scope of this PR and should be considered at a later time. Closes #1907. --------- Co-authored-by: Tom de Wit <tdewit@gainsight.com>
This PR:
In order to improve the developer experience this PR forces Mutators to include remedies in their definitions.