feat: add mutation testing for test quality assurance#505
Conversation
- pubspec.yaml: Add mutation_test ^1.8.0 to dev_dependencies - mutation_test.yaml: Configuration for mutation testing - docs/ci/mutation-testing.yaml: CI workflow template (copy to .github/workflows/) - docs/MUTATION_TESTING.md: Comprehensive documentation with usage guide The mutation testing infrastructure enables: - Mutation score measurement to evaluate test suite quality - Detection of weak tests that don't actually validate behavior - CI integration with full and incremental testing - Gradual improvement of test quality over time Note: The GitHub App cannot push workflow files directly. The CI workflow is provided in docs/ci/ and must be manually copied to .github/workflows/ by a maintainer.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis pull request introduces comprehensive mutation testing infrastructure for Mostro Mobile. It adds documentation in Spanish, GitHub Actions workflows for CI integration, a mutation testing configuration file, and the mutation_test dev dependency to pubspec.yaml. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
- mutation_test.yaml: Add mutation testing configuration - docs/ci/mutation-testing.yaml: CI workflow template - docs/MUTATION_TESTING.md: Comprehensive documentation - pubspec.yaml: Add mutation_test ^1.8.0 Fixes markdown lint warnings (MD040) by adding language specifiers to all code blocks. Note: CI workflow must be manually copied to .github/workflows/
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
docs/ci/mutation-testing.yaml (1)
66-77: Consider edge cases in changed files detection.The script works well for typical cases, but consider:
- If many files change, the command line in line 82 could exceed shell limits (though rare)
- The
grep -vchain filters correctly but could be simplified with a single patternThe current implementation is functional for typical PRs.
Optional: Simplify grep chain
- FILES=$(git diff --name-only origin/${{ github.base_ref }} -- 'lib/**/*.dart' | grep -v '_test.dart$' | grep -v '.g.dart$' | grep -v '.freezed.dart$' | grep -v '.gr.dart$' | tr '\n' ' ') + FILES=$(git diff --name-only origin/${{ github.base_ref }} -- 'lib/**/*.dart' | grep -vE '(_test|\.g|\.freezed|\.gr)\.dart$' | tr '\n' ' ')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/ci/mutation-testing.yaml` around lines 66 - 77, The "Get changed source files" step can fail on many-file PRs and the grep chain is verbose; update the step that sets FILES so it uses NUL-separated output and a single exclusion regex: run git diff --name-only -z origin/${{ github.base_ref }} -- 'lib/**/*.dart' piped to grep -zEv '(_test\.dart$|\.g\.dart$|\.freezed\.dart$|\.gr\.dart$)' and then safely join NUL-separated names into the FILES variable (or use xargs -0 to build the space-separated list) so you avoid shell argument length issues and replace the multiple grep -v filters with one grep -Ev pattern; keep the subsequent checks that write files=$FILES and skip=true/false unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/ci/mutation-testing.yaml`:
- Around line 13-17: The full mutation-test job is currently running on
pull_request events and duplicates the incremental mutation-test-pr job; update
the workflow so the mutation-test job does not run for PRs by removing or
excluding pull_request from its triggers (or add a conditional to skip when
github.event_name == "pull_request"), leaving mutation-test-pr as the sole PR
job; target the job named mutation-test and ensure only mutation-test-pr handles
pull_request events to prevent duplicate full+incremental runs and conflicting
reports.
- Around line 79-82: The PR job mutation-test-pr is missing the unit test step
before running mutation tests; add a step before the "Run mutation tests on
changed files" step that executes the same unit tests used by the main
mutation-test job (use "flutter test" as in the main job) so mutation testing
only runs if unit tests pass; update the job named "mutation-test-pr" to include
the flutter test step (mirroring the main job's test step) and ensure it runs
before the existing run: dart run mutation_test ${{ steps.changed.outputs.files
}} step.
In `@docs/MUTATION_TESTING.md`:
- Around line 125-129: The fenced code block in docs/MUTATION_TESTING.md lacks a
language specifier; update the opening triple-backtick for the block that
contains "# Mutation testing\nmutation-test-report/\n*.mutation-test-cache" to
include a language (preferably "gitignore" or "text") so it reads ```gitignore —
locate the fenced block in that file and replace the opening backticks
accordingly (the block shown under the Mutation testing heading).
- Around line 277-283: The fenced code block showing the sprint progression in
the docs/MUTATION_TESTING.md should include a language specifier for proper
rendering; update the triple-backtick fence that contains "Sprint 1: 50% ...
Target: 80% para módulos críticos" to use a language tag (e.g., add "text" after
the opening ```), so the block becomes ```text ... ``` to ensure correct
formatting.
- Around line 24-26: The fenced code block containing the formula "Score =
(Mutantes kill / Total mutantes) × 100" needs a language specifier; update the
triple-backtick fence that surrounds that line (the code block with the formula)
to use a language tag such as text or plaintext (e.g., ```text) so the markdown
follows the project's fenced code block guideline.
- Around line 182-197: The fenced code block that starts with "Found 45
mutations in 5 source files!" in MUTATION_TESTING.md needs a language specifier;
update the opening triple backticks for that block to include `text` (e.g.,
change ``` to ```text) so the block is rendered with the correct language
highlighting/formatting.
---
Nitpick comments:
In `@docs/ci/mutation-testing.yaml`:
- Around line 66-77: The "Get changed source files" step can fail on many-file
PRs and the grep chain is verbose; update the step that sets FILES so it uses
NUL-separated output and a single exclusion regex: run git diff --name-only -z
origin/${{ github.base_ref }} -- 'lib/**/*.dart' piped to grep -zEv
'(_test\.dart$|\.g\.dart$|\.freezed\.dart$|\.gr\.dart$)' and then safely join
NUL-separated names into the FILES variable (or use xargs -0 to build the
space-separated list) so you avoid shell argument length issues and replace the
multiple grep -v filters with one grep -Ev pattern; keep the subsequent checks
that write files=$FILES and skip=true/false unchanged.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docs/MUTATION_TESTING.mddocs/ci/mutation-testing.yamlmutation_test.yamlpubspec.yaml
| jobs: | ||
| mutation-test: | ||
| name: Run mutation tests | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 60 |
There was a problem hiding this comment.
Both jobs run on PRs, causing duplicate mutation testing.
The mutation-test job triggers on pull_request events (lines 6-7) and runs a full mutation test. Meanwhile, mutation-test-pr (line 49) also runs on PRs with incremental testing. This causes PRs to run both full AND incremental mutation tests, which:
- Wastes CI resources
- Produces conflicting reports (mutation-report vs mutation-report-pr)
Consider restricting the full mutation-test job to exclude PRs, or removing the duplicate PR trigger.
Proposed fix: Exclude PRs from full mutation test job
mutation-test:
name: Run mutation tests
+ if: github.event_name != 'pull_request'
runs-on: ubuntu-latest
timeout-minutes: 60🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/ci/mutation-testing.yaml` around lines 13 - 17, The full mutation-test
job is currently running on pull_request events and duplicates the incremental
mutation-test-pr job; update the workflow so the mutation-test job does not run
for PRs by removing or excluding pull_request from its triggers (or add a
conditional to skip when github.event_name == "pull_request"), leaving
mutation-test-pr as the sole PR job; target the job named mutation-test and
ensure only mutation-test-pr handles pull_request events to prevent duplicate
full+incremental runs and conflicting reports.
| - name: Run mutation tests on changed files | ||
| if: steps.changed.outputs.skip == 'false' | ||
| continue-on-error: true | ||
| run: dart run mutation_test ${{ steps.changed.outputs.files }} |
There was a problem hiding this comment.
PR mutation test missing unit test step.
The mutation-test-pr job skips running flutter test before mutation testing, unlike the main mutation-test job (line 30-31). If unit tests fail, mutation testing results would be misleading.
Proposed fix: Add unit test step
- name: Install dependencies
run: flutter pub get
+ - name: Run unit tests first
+ run: flutter test
+
- name: Get changed source files
id: changed🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/ci/mutation-testing.yaml` around lines 79 - 82, The PR job
mutation-test-pr is missing the unit test step before running mutation tests;
add a step before the "Run mutation tests on changed files" step that executes
the same unit tests used by the main mutation-test job (use "flutter test" as in
the main job) so mutation testing only runs if unit tests pass; update the job
named "mutation-test-pr" to include the flutter test step (mirroring the main
job's test step) and ensure it runs before the existing run: dart run
mutation_test ${{ steps.changed.outputs.files }} step.
| ``` | ||
| Score = (Mutantes kill / Total mutantes) × 100 | ||
| ``` |
There was a problem hiding this comment.
Add language specifier to fenced code block.
As per coding guidelines, all fenced code blocks in markdown files must have a language specifier. This formula block should use text or plaintext.
Proposed fix
-```
+```text
Score = (Mutantes kill / Total mutantes) × 100</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 24-24: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/MUTATION_TESTING.md` around lines 24 - 26, The fenced code block
containing the formula "Score = (Mutantes kill / Total mutantes) × 100" needs a
language specifier; update the triple-backtick fence that surrounds that line
(the code block with the formula) to use a language tag such as text or
plaintext (e.g., ```text) so the markdown follows the project's fenced code
block guideline.
| ``` | ||
| # Mutation testing | ||
| mutation-test-report/ | ||
| *.mutation-test-cache | ||
| ``` |
There was a problem hiding this comment.
Add language specifier to fenced code block.
The .gitignore content block needs a language specifier. Use gitignore or text.
Proposed fix
-```
+```gitignore
# Mutation testing
mutation-test-report/
*.mutation-test-cache</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 125-125: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/MUTATION_TESTING.md` around lines 125 - 129, The fenced code block in
docs/MUTATION_TESTING.md lacks a language specifier; update the opening
triple-backtick for the block that contains "# Mutation
testing\nmutation-test-report/\n*.mutation-test-cache" to include a language
(preferably "gitignore" or "text") so it reads ```gitignore — locate the fenced
block in that file and replace the opening backticks accordingly (the block
shown under the Mutation testing heading).
| ``` | ||
| Found 45 mutations in 5 source files! | ||
|
|
||
| lib/services/key_derivator.dart: 12 mutations | ||
| ✅ 10 killed (83.3%) | ||
| ❌ 2 survived (16.7%) | ||
|
|
||
| lib/features/auth/auth_service.dart: 8 mutations | ||
| ✅ 8 killed (100%) | ||
|
|
||
| Results: | ||
| Total: 45 mutations | ||
| Killed: 40 (88.9%) | ||
| Survived: 5 (11.1%) | ||
| Quality: B - Good | ||
| ``` |
There was a problem hiding this comment.
Add language specifier to fenced code block.
The example output block needs a language specifier. Use text or console.
Proposed fix
-```
+```text
Found 45 mutations in 5 source files!🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 182-182: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/MUTATION_TESTING.md` around lines 182 - 197, The fenced code block that
starts with "Found 45 mutations in 5 source files!" in MUTATION_TESTING.md needs
a language specifier; update the opening triple backticks for that block to
include `text` (e.g., change ``` to ```text) so the block is rendered with the
correct language highlighting/formatting.
| ``` | ||
| Sprint 1: 50% | ||
| Sprint 2: 55% | ||
| Sprint 3: 60% | ||
| ... | ||
| Target: 80% para módulos críticos | ||
| ``` |
There was a problem hiding this comment.
Add language specifier to fenced code block.
The sprint progression example needs a language specifier.
Proposed fix
-```
+```text
Sprint 1: 50%
Sprint 2: 55%
Sprint 3: 60%
...
Target: 80% para módulos críticos</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 277-277: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/MUTATION_TESTING.md` around lines 277 - 283, The fenced code block
showing the sprint progression in the docs/MUTATION_TESTING.md should include a
language specifier for proper rendering; update the triple-backtick fence that
contains "Sprint 1: 50% ... Target: 80% para módulos críticos" to use a language
tag (e.g., add "text" after the opening ```), so the block becomes ```text ...
``` to ensure correct formatting.
- Change index.html to mutation-test-report.html (actual filename)
All documentation, code comments, and GitHub content must be in English per project guidelines.
Summary
Add mutation testing infrastructure to evaluate test suite quality for Mostro Mobile, following the pattern established in grunch/choke#39.
Changes
mutation_test: ^1.8.0to dev_dependencieslib/.github/workflows/to activate)What is Mutation Testing?
Mutation testing evaluates the quality of your test suite by introducing small changes (mutations) to the source code and checking whether the tests detect them.
The mutation score (% killed) indicates how effective your tests are at catching real bugs.
Benefits for Mostro Mobile
Mostro Mobile handles critical Bitcoin P2P trading - high-quality tests are essential:
CI Integration
After copying
docs/ci/mutation-testing.yamlto.github/workflows/mutation-testing.yaml, the workflow runs:mainand weekly (Monday 6:00 UTC)Both jobs are non-blocking initially (
continue-on-error: true). To enforce a minimum mutation score, set thefailurethreshold inmutation_test.yaml.Testing Locally
Related
Acceptance Criteria
Summary by CodeRabbit
Documentation
Chores