Is the infection dependency necessary? about composer-stager

[TravisCarden]

No, it is not running anywhere automatically. I use it ad hoc and haven't looked into CI integration. But I would be open to it. Do you have any thoughts, @alexpott?

[effulgentsia]

@TravisCarden: can you write up how you currently use it ad hoc?

[TravisCarden]

Sure, @effulgentsia. I generally run it after I've completed a feature to help me identify weaknesses in my tests, like so:

$ composer infection
> infection

    ____      ____          __  _
   /  _/___  / __/__  _____/ /_(_)___  ____
   / // __ \/ /_/ _ \/ ___/ __/ / __ \/ __ \
 _/ // / / / __/  __/ /__/ /_/ / /_/ / / / /
/___/_/ /_/_/  \___/\___/\__/_/\____/_/ /_/

#StandWithUkraine

Infection - PHP Mutation Testing Framework version 0.26.6


Running initial test suite...

PHPUnit version: 9.5.27

  241 [============================] 32 secs

Generate mutants...

Processing source code files: 75/75
.: killed, M: escaped, U: uncovered, E: fatal error, X: syntax error, T: timed out, S: skipped, I: ignored

UUU..........................MMM..................   ( 50 / 239)
.................MMMM....M........................   (100 / 239)
...............................................MM.   (150 / 239)
..............MM............M.MM............M.....   (200 / 239)
..M.....M..............................              (239 / 239)

239 mutations were generated:
     218 mutants were killed
       0 mutants were configured to be ignored
       3 mutants were not covered by tests
      18 covered mutants were not detected
       0 errors were encountered
       0 syntax errors were encountered
       0 time outs were encountered
       0 mutants required more time than configured

Metrics:
         Mutation Score Indicator (MSI): 91%
         Mutation Code Coverage: 98%
         Covered Code MSI: 92%

Please note that some mutants will inevitably be harmless (i.e. false positives).

Time: 1m 48s. Memory: 28.00MB

It saves defails to var/infection/infection.log, which I examine for issues. Here's a snippet from the above run:

Escaped mutants:
================

1) src/Domain/Exception/PreconditionException.php:21    [M] DecrementInteger

--- Original
+++ New
@@ @@
 class PreconditionException extends RuntimeException implements ExceptionInterface
 {
     private PreconditionInterface $precondition;
-    public function __construct(PreconditionInterface $precondition, string $message = '', int $code = 0, ?Throwable $previous = null)
+    public function __construct(PreconditionInterface $precondition, string $message = '', int $code = -1, ?Throwable $previous = null)
     {
         $this->precondition = $precondition;
         parent::__construct($message, $code, $previous);

It's all strictly heuristic--as its output says, "some mutants will inevitably be harmless (i.e. false positives)"--so I use it as a guide, not a gate. Besides, I haven't figured out how to "fix" all the issues it finds or thought about how to decide which results constitute false positives and suppress them.

Having "talked" it through, though, I can see how it would be valuable to have the output in CI builds, and I would be very open to adding a job for it. I would also be happy to discuss and document standards for using it.

[xjm]

I would encourage us to have fewer dev dependencies, not more, especially given the false positives which will create overhead. The dependency policy at https://github.com/php-tuf/php-tuf#dependency-policies-and-information was intended to apply to all projects in the namespace, but it looks like the practice was not applied to this package.

[TravisCarden]

I've removed phpmetrics/phpmetrics, which was an easy decision. I could eliminate two more with some test refactoring. @effulgentsia and I are going to talk about the rest.

[effulgentsia]

The dependency policy at https://github.com/php-tuf/php-tuf#dependency-policies-and-information was intended to apply to all projects in the namespace, but it looks like the practice was not applied to this package.

I'm unclear on how to interpret that policy with respect to dev dependencies. The above link links to https://www.drupal.org/about/core/policies/core-dependency-policies/dependency-info. That page says to evaluate dependency changes with composer-lock-diff, but that only looks at a package's production dependencies. In other words, if I start with a git checkout of Drupal core 10.1.x, and then do:

composer require php-tuf/composer-stager
composer-lock-diff

All that outputs is:

+-------------------------+------+--------+---------+
| Production Changes      | From | To     | Compare |
+-------------------------+------+--------+---------+
| php-tuf/composer-stager | NEW  | v1.2.0 |         |
| symfony/filesystem      | NEW  | v6.2.0 |         |
+-------------------------+------+--------+---------+

+--------------------+--------+---------+---------+
| Dev Changes        | From   | To      | Compare |
+--------------------+--------+---------+---------+
| symfony/filesystem | v6.2.0 | REMOVED |         |
+--------------------+--------+---------+---------+

In other words, all it picks up is that core's dev dependency on symfony/filesystem gets promoted to a production dependency (because of php-tuf/composer-stager). #60 is the issue for removing Composer Stager's dependency on symfony/filesystem.

composer-lock-diff does not pick up any of Composer Stager's dev dependencies, because the dev dependencies of package B do not become dev dependencies of package A even if package A depends on package B.

Is the idea, however, that because https://www.drupal.org/project/drupal/issues/3331078 is proposing to place Composer Stager into Drupal core's maintenance/governance, that we should evaluate all of Composer Stager's dev dependencies the same way that we would evaluate them if they were being proposed as dev dependencies for Drupal core? In which case, yeah, I think we should open up an issue for each of Composer Stager's current dev dependencies to discuss which to keep and which to remove. We already have this one for infection/infection.

[TravisCarden]

I've decided to go ahead and remove infection/infection. I get value from it, but probably enough to warrant its inclusion at present. I can probably install it globally on my machine to use on an ad-hoc basis. (Other dev dependencies are being considered in #78.)