Add mutation testing

[TheBlueMatt]

We should start requiring full mutation coverage for functions as we build out our unit tests.
https://github.com/llogiq/mutagen looks like a good candidate, unless @apoelstra thinks we should use https://github.com/apoelstra/halfsleep.
Tagging 0.2 as our goal for 0.2 should be to move more towards better test coverage.

[apoelstra]

Nice!

halfsleep is unmaintained, and I'd probably start contributing to mutagen before I'd start maintaining it again.

[TheBlueMatt]

There’s a new option on the block though this one is language-agnostic and uses LLVM. No idea what the integration work would look like and it may end up just generating a ton of false positives for things in stdlib instead of our own code. https://github.com/mull-project/mull

…

On Sep 19, 2018, at 06:43, Andrew Poelstra ***@***.***> wrote:  Nice! halfsleep is unmaintained, and I'd probably start contributing to mutagen before I'd start maintaining it again. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

[dunxen]

So there's an even newer option now called cargo-mutants that doesn't require modifying any part of the source tree permanently. It's just run as a cargo subcommand which is convenient. I'm running it on main now to see if we get anything useful. I'm already getting a few missed mutants which could indicate some gaps already found. :)

[apoelstra]

cc @tcharding if you're bored and want to play with this one, you might beat me to it:)

[tcharding]

Sweet, cheers. Do note before you put an policy in place I've found mutation testing enjoyable because one really has to understand all the code tested but it is slow to implement in my experience.

[dunxen]

Mutating all functions over everything took about 12 hours (without --jobs for spawning multiple cargo processes) for me.
2421 mutants tested in 777:57: 505 missed, 1122 caught, 778 unviable, 16 timeouts
So I'm not sure what would be the best way to schedule something like this or maybe set it to run more granularly for PRs. One can filter by functions introduced in a PR. That might be more palatable for CI. Next would be figuring out the best way to use the results.

[TheBlueMatt]

Awesome! That's cool. I'm glad there's more mutation testing projects in Rust. As for what to do with the results, I think we should hard fail if new functions can be replaced with dummies with no test failure (Iiuc this is all the cargo mutants project does, no if inversion or so). It may well be very loud, but we can revert it if we get annoyed, and we don't need to require it for merges just yet, first just show a red X.

[tcharding]

I haven't played with cargo mutate yet but mutagen gives false positives is some cases, I don't think you'll be able to hard fail. An example I have been hitting often is < vs <=

fn abs(x: i32) -> u32 {
    if x < 0 { -x } else { x };
}

[dunxen]

I haven't played with cargo mutate yet but mutagen gives false positives is some cases, I don't think you'll be able to hard fail. An example I have been hitting often is < vs <=

fn abs(x: i32) -> u32 {

    if x < 0 { -x } else { x };

}

Yeah, in this case then you'd need to add an ignore attribute to that function. So I think hard fail is still fine and the author can just explicitly ignore that function

[tcharding]

That is definitely one solution, I love to find one that didn't use ignore because I've found false positives pretty common but there is still some advantage in mutating. I'm super new to mutation testing but if you guys come up with anything can you cc me please. I did suggest on the mutagen repo adding an attribute like rustfmt::skip, it was favorably received but I wasn't able to implement it in the one try I had at it.

[TheBlueMatt]

An example I have been hitting often is < vs <=

From reading the cargo-mutants doc I dont think it does anything like that (it only shows 2421 mutants across all of RL, so its not doing much :) ) - it looks like all it does is check that, for each function, if you can replace it with a completely dummy function returning a static value, it checks that your tests fail. At that level of granularity I feel like showing a CI "X" is reasonable, even if we don't strictly require it for merging.

[tcharding]

Oh, thanks for the explanation - it is definitely not a total replacement for mutagen then. I ran it on rust-bitcoin yesterday but haven't gone through the output yet.

[dunxen]

At that level of granularity I feel like showing a CI "X" is reasonable, even if we don't strictly require it for merging.

I think this would also be an important feature sourcefrog/cargo-mutants#57, but I don't think it's a hard requirement to at least test it out. I'll play around some more and open a PR at some stage. Also seeing what I can contribute upstream.

[sourcefrog]

Hi, just thought I would let you know cargo-mutants can now do incremental tests on PRs, in https://mutants.rs/in-diff.html. Let me know over there if you have any feedback or hit any problems.

[dunxen]

Hi, just thought I would let you know cargo-mutants can now do incremental tests on PRs

Hey! Thanks, that's awesome. I'll be sure to give it a try soon and let you know how it goes 😄

[tnull]

Closing as initial support was added in #2763.