Also mutate > to >= and < to <=

[nfrankel]

Most tests miss testing the boundary. Mutation testing can help a lot with this. The current code misses the change from > to >= and < and <=.

[nfrankel]

The failing test checks for a typo on a source file I didn't touch.

[sourcefrog]

Hi, thanks for the patch.

I think this is worth considering, but we ought to check whether it generates too many low-value hits. Have you tried it on a large tree.

[nfrankel]

I leave it up to you, but this mutation is part of established mutation testing tools.

https://pitest.org/quickstart/mutators/#CONDITIONALS_BOUNDARY

[sourcefrog]

I know, but it would also be useful to know that you tried it on some tree and it found a certain fraction of interesting or uninteresting misses.

[nfrankel]

I didn't try it on a regular codebase. I just wanted to test this specific use-case, which is the one where Mutation Testing shines, and found it lacking.

[sourcefrog]

Next steps:

• Aside from the typo warning, which I fixed in main, there are some tests failing due to this generating new mutants that are not expected by the tests
• After the tests pass, mutants test the mutants tree and see if anything is caught, and whether they seem like interesting findings
• book/src/mutants.md should be updated to describe this
• Add it to NEWS

[nfrankel]

I did my best, but I'm stuck on one of the tests in error. I can't understand where I should set the expectation.

[sourcefrog]

Thanks, I'll look at the tests, I'm just busy with other things this week.

[nfrankel]

No worries, there is no hurry

[sourcefrog]

The tests are failing in part because => is not a comparison operator in Rust, it's a double arrow.

(Also, there are two tests called check_tree_with_mutants_skip which probably makes things confusing. I'll fix that on main.)

[sourcefrog]

With the tests fixed this does find some missing mutations in this tree:

MISSED   src/tail_file.rs:44:19: replace > with >= in TailFile::last_line in 1.0s build + 50.9s test
MISSED   src/path.rs:24:19: replace > with >= in ascent in 1.0s build + 50.2s test

In the first, it's somewhat usefully pointing out redundant code that could be removed.

The second is a bit harder:

        if ascent > max_ascent {
            max_ascent = ascent;
        }

It's true that no test will notice if > is replaced with >= here, but it's also not a bug to do so...

[sourcefrog]

Actually, even though that mutant is marginal value, rewriting the code to avoid it did find some untested paths, so on the whole perhaps it's a good suggestion.

[sourcefrog]

Merged in #503, thanks!

[nfrankel]

Thanks! You did most of the work. I only added the embryo.