Skip to content

Commit bb6e887

Browse files
ASuciuXASuciuX
authored
Merge pull request #4178 from ASuciuX/test/mutants-filter-pr
Mutation testing - pr differences
2 parents 4b6638a + c16d87c commit bb6e887

File tree

2 files changed

+234
-0
lines changed

2 files changed

+234
-0
lines changed

‎.github/workflows/pr-differences-mutants.yml

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
name: PR Differences Mutants
2+
3+
on:
4+
pull_request:
5+
types:
6+
- opened
7+
- reopened
8+
- synchronize
9+
- ready_for_review
10+
paths:
11+
- "**.rs"
12+
13+
concurrency:
14+
group: pr-differences-${{ github.head_ref || github.ref || github.run_id }}
15+
# Always cancel duplicate jobs
16+
cancel-in-progress: true
17+
18+
jobs:
19+
# Check and output whether to run big (`stacks-node`/`stackslib`) or small (others) packages with or without shards
20+
check-big-packages-and-shards:
21+
name: Check Packages and Shards
22+
23+
runs-on: ubuntu-latest
24+
25+
outputs:
26+
run_big_packages: ${{ steps.check_packages_and_shards.outputs.run_big_packages }}
27+
big_packages_with_shards: ${{ steps.check_packages_and_shards.outputs.big_packages_with_shards }}
28+
run_small_packages: ${{ steps.check_packages_and_shards.outputs.run_small_packages }}
29+
small_packages_with_shards: ${{ steps.check_packages_and_shards.outputs.small_packages_with_shards }}
30+
31+
steps:
32+
- id: check_packages_and_shards
33+
uses: stacks-network/actions/stacks-core/mutation-testing/check-packages-and-shards@main
34+
35+
# Mutation testing - Execute on PR on small packages that have functions modified (normal run, no shards)
36+
pr-differences-mutants-small-normal:
37+
name: Mutation Testing - Normal, Small
38+
39+
needs: check-big-packages-and-shards
40+
41+
if: ${{ needs.check-big-packages-and-shards.outputs.run_small_packages == 'true' && needs.check-big-packages-and-shards.outputs.small_packages_with_shards == 'false' }}
42+
43+
runs-on: ubuntu-latest
44+
45+
steps:
46+
- name: Run mutants on diffs
47+
uses: stacks-network/actions/stacks-core/mutation-testing/pr-differences@main
48+
with:
49+
package-dimension: "small"
50+
51+
# Mutation testing - Execute on PR on small packages that have functions modified (run with strategy matrix shards)
52+
pr-differences-mutants-small-shards:
53+
name: Mutation Testing - Shards, Small
54+
55+
needs: check-big-packages-and-shards
56+
57+
if: ${{ needs.check-big-packages-and-shards.outputs.run_small_packages == 'true' && needs.check-big-packages-and-shards.outputs.small_packages_with_shards == 'true' }}
58+
59+
runs-on: ubuntu-latest
60+
61+
strategy:
62+
fail-fast: false
63+
matrix:
64+
shard: [0, 1, 2, 3]
65+
66+
steps:
67+
- name: Run mutants on diffs
68+
uses: stacks-network/actions/stacks-core/mutation-testing/pr-differences@main
69+
with:
70+
shard: ${{ matrix.shard }}
71+
package-dimension: "small"
72+
73+
# Mutation testing - Execute on PR on big packages that have functions modified (normal run, no shards)
74+
pr-differences-mutants-big-normal:
75+
name: Mutation Testing - Normal, Big
76+
77+
needs: check-big-packages-and-shards
78+
79+
if: ${{ needs.check-big-packages-and-shards.outputs.run_big_packages == 'true' && needs.check-big-packages-and-shards.outputs.big_packages_with_shards == 'false' }}
80+
81+
runs-on: ubuntu-latest
82+
83+
steps:
84+
- name: Run Run mutants on diffs
85+
env:
86+
BITCOIND_TEST: 1
87+
RUST_BACKTRACE: full
88+
uses: stacks-network/actions/stacks-core/mutation-testing/pr-differences@main
89+
with:
90+
package-dimension: "big"
91+
92+
# Mutation testing - Execute on PR on big packages that have functions modified (run with strategy matrix shards)
93+
pr-differences-mutants-big-shards:
94+
name: Mutation Testing - Shards, Big
95+
96+
needs: check-big-packages-and-shards
97+
98+
if: ${{ needs.check-big-packages-and-shards.outputs.run_big_packages == 'true' && needs.check-big-packages-and-shards.outputs.big_packages_with_shards == 'true' }}
99+
100+
runs-on: ubuntu-latest
101+
102+
strategy:
103+
fail-fast: false
104+
matrix:
105+
shard: [0, 1, 2, 3, 4, 5, 6, 7]
106+
107+
steps:
108+
- name: Run mutants on diffs
109+
env:
110+
BITCOIND_TEST: 1
111+
RUST_BACKTRACE: full
112+
uses: stacks-network/actions/stacks-core/mutation-testing/pr-differences@main
113+
with:
114+
shard: ${{ matrix.shard }}
115+
package-dimension: "big"
116+
117+
# Output the mutants and fail the workflow if there are missed/timeout/unviable mutants
118+
output-mutants:
119+
name: Output Mutants
120+
121+
runs-on: ubuntu-latest
122+
123+
needs:
124+
[
125+
check-big-packages-and-shards,
126+
pr-differences-mutants-small-normal,
127+
pr-differences-mutants-small-shards,
128+
pr-differences-mutants-big-normal,
129+
pr-differences-mutants-big-shards,
130+
]
131+
132+
steps:
133+
- name: Output Mutants
134+
uses: stacks-network/actions/stacks-core/mutation-testing/output-pr-mutants@main
135+
with:
136+
big_packages: ${{ needs.check-big-packages-and-shards.outputs.run_big_packages }}
137+
shards_for_big_packages: ${{ needs.check-big-packages-and-shards.outputs.big_packages_with_shards }}
138+
small_packages: ${{ needs.check-big-packages-and-shards.outputs.run_small_packages }}
139+
shards_for_small_packages: ${{ needs.check-big-packages-and-shards.outputs.small_packages_with_shards }}

‎docs/ci-release.md

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,4 +228,99 @@ ex: Branch is named `develop` and the PR is numbered `113`
228228
- `stacks-core:2.1.0.0.0`
229229
- `stacks-core:latest`
230230

231+
## Mutation Testing
232+
233+
When a new Pull Request (PR) is submitted, this feature evaluates the quality of the tests added or modified in the PR.
234+
It checks the new and altered functions through mutation testing.
235+
Mutation testing involves making small changes (mutations) to the code to check if the tests can detect these changes.
236+
237+
The mutations are run with or without a [Github Actions matrix](https://docs.github.com/en/actions/using-jobs/using-a-matrix-for-your-jobs).
238+
The matrix is used when there is a large number of mutations to run ([check doc specific cases](https://github.com/stacks-network/actions/blob/main/stacks-core/mutation-testing/check-packages-and-shards/README.md#outputs)).
239+
We utilize a matrix strategy with shards to enable parallel execution in GitHub Actions.
240+
This approach allows for the concurrent execution of multiple jobs across various runners.
241+
The total workload is divided across all shards, effectively reducing the overall duration of a workflow because the time taken is approximately the total time divided by the number of shards (+ initial build & test time).
242+
This is particularly advantageous for large packages that have significant build and test times, as it enhances efficiency and speeds up the process.
243+
244+
Since mutation testing is directly correlated to the written tests, there are slower packages (due to the quantity or time it takes to run the tests) like `stackslib` or `stacks-node`.
245+
These mutations are run separately from the others, with one or more parallel jobs, depending on the amount of mutations found.
246+
247+
Once all the jobs have finished testing mutants, the last job collects all the tested mutations from the previous jobs, combines them and outputs them to the `Summary` section of the workflow, at the bottom of the page.
248+
There, you can find all mutants on categories, with links to the function they tested, and a short description on how to fix the issue.
249+
The PR should only be approved/merged after all the mutants tested are in the `Caught` category.
250+
251+
### Time required to run the workflow based on mutants outcome and packages' size
252+
253+
- Small packages typically completed in under 30 minutes, aided by the use of shards.
254+
- Large packages like stackslib and stacks-node initially required about 20-25 minutes for build and test processes.
255+
- Each "missed" and "caught" mutant took approximately 15 minutes. Using shards, this meant about 50-55 minutes for processing around 32 mutants (10-16 functions modified). Every additional 8 mutants added another 15 minutes to the runtime.
256+
- "Unviable" mutants, which are functions lacking a Default implementation for their returned struct type, took less than a minute each.
257+
- "Timeout" mutants typically required more time. However, these should be marked to be skipped (by adding a skip flag to their header) since they indicate functions unable to proceed in their test workflow with mutated values, as opposed to the original implementations.
258+
259+
File:
260+
261+
- [PR Differences Mutants](../.github/workflows/pr-differences-mutants.yml)
262+
263+
### Mutant Outcomes
264+
265+
- caught — A test failed with this mutant applied.
266+
This is a good sign about test coverage.
267+
268+
- missed — No test failed with this mutation applied, which seems to indicate a gap in test coverage.
269+
Or, it may be that the mutant is undistinguishable from the correct code.
270+
In any case, you may wish to add a better test.
271+
272+
- unviable — The attempted mutation doesn't compile.
273+
This is inconclusive about test coverage, since the function's return structure may not implement `Default::default()` (one of the mutations applied), hence causing the compile to fail.
274+
It is recommended to add `Default` implementation for the return structures of these functions, only mark that the function should be skipped as a last resort.
275+
276+
- timeout — The mutation caused the test suite to run for a long time, until it was eventually killed.
277+
You might want to investigate the cause and only mark the function to be skipped if necessary.
278+
279+
### Skipping Mutations
280+
281+
Some functions may be inherently hard to cover with tests, for example if:
282+
283+
- Generated mutants cause tests to hang.
284+
- You've chosen to test the functionality by human inspection or some higher-level integration tests.
285+
- The function has side effects or performance characteristics that are hard to test.
286+
- You've decided that the function is not important to test.
287+
288+
To mark functions as skipped, so they are not mutated:
289+
290+
- Add a Cargo dependency of the [mutants](https://crates.io/crates/mutants) crate, version `0.0.3` or later (this must be a regular `dependency`, not a `dev-dependency`, because the annotation will be on non-test code) and mark functions with `#[mutants::skip]`, or
291+
292+
- You can avoid adding the dependency by using the slightly longer `#[cfg_attr(test, mutants::skip)]`.
293+
294+
### Example
295+
296+
```rust
297+
use std::time::{Duration, Instant};
298+
299+
/// Returns true if the program should stop
300+
#[cfg_attr(test, mutants::skip)] // Returning false would cause a hang
301+
fn should_stop() -> bool {
302+
true
303+
}
304+
305+
pub fn controlled_loop() {
306+
let start = Instant::now();
307+
for i in 0.. {
308+
println!("{}", i);
309+
if should_stop() {
310+
break;
311+
}
312+
if start.elapsed() > Duration::from_secs(60 * 5) {
313+
panic!("timed out");
314+
}
315+
}
316+
}
317+
318+
mod test {
319+
#[test]
320+
fn controlled_loop_terminates() {
321+
super::controlled_loop()
322+
}
323+
}
324+
```
325+
231326
---

0 commit comments

Comments
 (0)