LibreSign / LibreSign/libresign

Improve PHP tests with Infection mutation testing

Open
#8,053 15 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

backend good first issue php
Dominant language
PHP
Stars
818
Forks
146
Avg merge
10h 53m
Merged PRs (30d)
353

Description

LibreSign now uses Infection to help check the quality of the PHP tests.

Code coverage tells us which code was executed by tests. However, it does not tell us if the tests can detect wrong behavior. Mutation testing helps us check this.

Infection makes small changes to the source code. These changes are called mutants. After each change, Infection runs the tests again:

  • If a test fails, the mutant is killed. This is good because the test detected the change.
  • If all tests still pass, the mutant escaped. This can mean that an important behavior is not well tested.
  • A mutant can also cause a timeout or an error. These cases need to be checked before deciding what should be changed.

The first full LibreSign run generated more than 15,000 mutants and took a long time to run. It also found many escaped mutants, errors and timeouts.

Initial run:

https://github.com/LibreSign/libresign/actions/runs/32801594308/job/97663483870

Because of this, the full Infection suite is not ready to be required in CI yet.

This issue tracks the work to improve the PHP code and tests step by step.

[!IMPORTANT]
Do not solve this issue in one PR.

This issue is open to many contributions.

Each PR should work on only one or two related PHP source/test pairs.

For example:

lib/Service/ExampleService.php
tests/php/Unit/Service/ExampleServiceTest.php

A PR can include two pairs when the classes are small and closely related.

Keep the change focused. As a general rule, try to keep the diff below about 1,000 lines. If it gets close to 2,000 lines, split the work into another PR.

Partial PRs should reference this issue, but should not use Fixes #... or Closes #... while there is still work to do.

Before starting, please comment on this issue with the source file you want to work on. This helps avoid two contributors working on the same code.

Effort

The complete issue is a high-effort task because it covers many PHP files and tests.

However, each contribution should be low to medium effort. A PR should focus on only one or two related source/test pairs.

You do not need to solve the complete issue. Small and focused improvements are welcome and can be merged separately.

What should be improved first

Please use this order when choosing what to work on:

  1. Timeouts or unsafe side effects found by Infection. These can make Infection slow or unstable and should be checked first.
  2. Escaped mutants in important business rules, especially signing, certificates, policies, permissions, validation and identity flows.
  3. Escaped mutants in other services and controllers.
  4. Other useful code or test improvements found while working on the selected file.

A fatal error caused by a mutant does not always mean that a test needs to change. Check the reason first.

Do not change tests only to make the Infection result look better.

How to work on one file

[!TIP]
If your development environment uses Docker, such as the LibreCode or Nextcloud development environments, composer may only be available inside the application container. Run the commands below inside the container or use the command provided by your development environment.

Start with one source file and its matching PHPUnit test.

For example:

composer test:unit -- --filter ExampleServiceTest

Then run Infection only for the selected source file:

composer mutation:test -- \
  lib/Service/ExampleService.php \
  --show-mutations

[!TIP]
Pass the source file directly to Infection. Do not use Infection's deprecated --filter option.

For each escaped mutant, ask:

What behavior changed, and why did the current test not detect it?

Then improve the code or add a test that checks this behavior.

The goal is not only to increase the Infection score. The goal is to make the LibreSign code and tests more reliable.

Code and test requirements

Changes should protect real behavior and business rules.

When working on this issue:

  • Follow the test conventions documented in AGENTS.md.
  • Keep the mirrored test structure:
    lib/.../Class.phptests/php/Unit/.../ClassTest.php.
  • First understand the behavior that needs to be protected.
  • When possible, add or improve the test before changing production code.
  • Test important success, failure, boundary and invalid-input cases.
  • Prefer data providers when the same business rule needs to be tested with different inputs or expected results.
  • Keep test methods small and focused. A test should normally check one clear case or rule.
  • Use clear test names that explain the expected behavior.
  • Avoid using Reflection only to access private methods or properties. Prefer testing behavior through the public API.
  • Use mocks when they represent a real dependency or when the interaction with a dependency is what the test needs to check.
  • Do not mock simple objects only because this makes the test easier to write.
  • Avoid tests that need many mocks. If a simple rule needs many mocks, check if a small code refactor can make the class easier to test.
  • Small production-code refactors are allowed when they improve testability. They must keep the same behavior and stay inside the selected scope.
  • Do not add broad exclusions, disable mutators, lower quality gates or add meaningless assertions only to improve the mutation score.
  • Do not use broad exception handling only to make code or tests pass. Check expected conditions directly and handle known errors where they happen.
  • Avoid generic catches such as catch (\Throwable) or catch (\Exception) when a more specific exception can be handled.
  • Use try/catch when there is a clear reason, such as recovery, cleanup or converting one error into another. It should not hide unexpected errors or replace proper validation.
  • In tests, prefer PHPUnit exception assertions when the expected result is a specific exception. Use try/catch only when the exception itself needs to be checked in more detail.

Try to get a very high Covered Code MSI for the selected scope, ideally 100%.

However, useful tests are more important than reaching 100%.

If a mutant cannot be killed with a meaningful test, explain the case in the PR. Do not change a test only to improve the score.

Do not ignore or exclude mutants, disable mutators, or change the Infection configuration only to make the result pass. If there is a strong technical reason for this type of change, discuss it with the maintainers first.

Using data providers

Use data providers when several cases test the same rule.

For example, instead of creating many large test methods for different invalid values, a data provider can list these cases and keep the test method small.

Do not put unrelated cases in the same data provider only to reduce the number of test methods. All datasets in a data provider should test the same behavior.

Architecture improvements

Mutation testing can also show code that is difficult to test.

For example, a class may have too many responsibilities or may call the operating system or another external service directly.

In these cases, a small refactor can be part of the PR if it makes the code easier and safer to test.

For example, an external or operating-system action can sometimes be moved behind a small method or dependency. This allows the business rule to be tested without running the real side effect.

Do not make a large production-code refactor as part of this issue.

If a large architecture change is needed, explain it in the PR and create a separate issue.

Acceptance criteria for each PR

  • The PR covers no more than two related source/test pairs.
  • The affected PHPUnit tests pass.
  • Infection is run only for the selected source files during development.
  • Escaped mutants in the selected scope are reviewed.
  • Tests are added or improved for important behavior that was not detected before.
  • Important business rules are tested through real behavior, not private implementation details.
  • Similar cases use data providers when this makes the tests easier to read.
  • Test methods are focused and reasonably small.
  • Reflection is not added only to access private implementation details.
  • Mocks are used only when they have a clear purpose.
  • No mutator or production code is excluded only to improve the score.
  • Any important escaped mutants that remain are explained in the PR.
  • Production-code refactors, when needed, are small, tested and keep the existing behavior.
  • The PR follows the LibreSign test structure and contribution rules.

References

Good first issue

You do not need to understand the complete LibreSign codebase or fix all mutation test results.

Choose one small PHP class. Understand what it should do, read its existing tests and improve only that part.

This is a good way to learn PHPUnit, mutation testing and LibreSign business rules while working on a small contribution that can be reviewed separately.

If you are not sure which file to choose, ask in this issue. A maintainer can help you choose a good starting point.

Additional context
  • If you have questions, feel free to ask in this issue.
  • Give a ⭐️ star to this repository if you find LibreSign useful and would like to support the project.
  • You can also join our community: https://t.me/LibreSign

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reading AGENTS.md, then choose one PHP source file and its mirrored PHPUnit test under lib/... and tests/php/Unit/.... Run the selected test with composer test:unit and Infection on the source file with composer mutation:test. Review escaped mutants and improve meaningful behavior coverage while keeping the work to no more than two related source/test pairs.

Written by the indexing model from the issue text.

Assessment

Tech stack
php
Domain
testing
Issue type
Refactor
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.