Current state of tests is not great, let's fix them

[s-kovacevic]

As the title says current state of tests is not where it should be especially considering the purpose of the mutmut itself is to improve unit tests. I feel that we need to address these issues sooner rather than later since it will make maintenance much easier in the future.

1. Current tests are more integration / end-to-end tests than unit tests
2. Big chunk of code is not properly unit tested
3. mutmut is not used on mutmut

Current tests are more integration / end-to-end tests than unit tests Here is the article by Michael Feathers that pretty much explains this point. Each unit test should test only one unit of the code, also no spinning disks for unit tests and no IO at all. Some parts of the code will, for sure, require refactoring to make them testable. There is a lot of IO in functions that should be moved up a level to make the code easy to test. Here is a great talk by Brandon Rhodes on how to "hoist IO".

Big chunk of code is not properly unit tested It is hard to determine actual unit test coverage since coverage is calculated while running all tests. Currently it looks good, but there is a decent amount of edge cases that are not tested because it is hard to reproduce edge cases by writing integration tests. Coverage should be calculated when running only true unit tests.

mutmut is not used on mutmut This is the best way to make our unit tests better, that's what mutmut does. Also this would make it easier for us to spot new use cases and potential improvements. But first, things need to be refactored.

Steps to mitigate:

• Making a clear separation between unit tests and integration / end-to-end tests. (different dirs/files and pytestmark)
• Stop merging pull requests that aren't properly covered with unit tests.
• Chunk by chunk refactor code to be testable and add appropriate unit tests for it.
• After the old code is refactored, switch coverage to work only with unit tests.
• Start using mutmut on unit tests

Good thing is that code base is not that big, it is a great time to do something like this. Also current tests are good for what they do, we just need to add proper unit tests.

Please let me know what you think. :)

[boxed]

I don't think we need to be religious about this. Mutmut is a tool that is very low impact if there are issues with it. I've thought about this a bit and it's ironic that mutmut isn't mutation tested, but in fact it doesn't make much sense to do so I think. Mutation testing is for critical pieces of code, and mutmut just isn't critical.

I think I mostly agree with "Write tests. Not too many. Mostly integration" (https://twitter.com/rauchg/status/807626710350839808), at least for mutmut. For some libraries that is dead wrong though and there you need unit tests plus mutation testing. It's all about context.

I agree though that all the tests of mutmut that use the disk are super annoying to test, but if we remove the disk access we just aren't testing the code anymore. We need to keep the tests we have.

mutmut/__init__.py could maybe be better tested with mutation testing, but cache.py and __main__.py only makes sense to test with end to end tests imo. And we should certainly not have mutation testing for stuff like the html reporting feature we've discussed before, that's just a waste of time.

[nklapste]

I agree with @s-kovacevic it would be nice to have our code more unit testable. Even if the majority of functionality can be tested from integration.

I think refactoring chunk by chunk should be a process we go forward with. Right now __init__.py and __main__.py have lots responsibilities, are tightly coupled, and can be quite a monolith to fully understand. We should adopt a structure that has a better separation of concerns (i.e mutation creation, test execution, source file collection, AST pattern matching). This could be done by creating new modules such as mutator.py, runner.py, collector.py, ast_matcher.py ect...

Having unit tests for each of these modules would then help illustrate responsibilities of each module. The current integration-like tests fail to do this effectively.

[boxed]

What would collector.py do?

[nklapste]

What would collector.py do?

I would put the functionality for collecting source files to mutate and the filtering out of test files there. Though, really that functionality is really only utilized in __main__.py anyways.

I try to follow a pattern were stuff that could have use being used programmatically (inside or outside of the project) gets its own module. While stuff that does the boilerplate and setup for a CLI entrypoint gets thrown in __main__.

[boxed]

I try to follow a pattern were stuff that could have use being used programmatically (inside or outside of the project) gets its own module.

Well then you end up with either having an awkward API (from mutmut.collect import collect) or having to put empty imports that just export symbols in __init__. I don't like that.

[nklapste]

I try to follow a pattern were stuff that could have use being used programmatically (inside or outside of the project) gets its own module.

Well then you end up with either having an awkward API (from mutmut.collect import collect) or having to put empty imports that just export symbols in __init__. I don't like that.

I agree empty imports hurt my soul.

File collection should likely be internal usage only anyways. I could see it being packed into __main__ without issue.

Though, some restructuring can still be done. Some of the functionality within __main__ such as the state management and running of tests via the methods: run_mutation_tests_for_file run_mutation_tests run_mutation tests_pass time_test_suite could be moved into a class such as a MutationTestRunner or Runner, thus, removing the need of a config object that gets bounced around multiple methods. Having all this state management contained in class keeps responsibility clear in my opinion makes understanding mutmut easier.

I could also see a similar structure being developed for the creation, and application of mutants in a Mutator class.

I have a branch at https://github.com/nklapste/mutmut/tree/prototype/muckup that followed this pattern and I thought it was pretty clean. Though, this branch did do feature removal and attempted to rebuild mutmut from the ground up.

You should look at it and If you like that pattern I could take another spin at refactoring mutmut to follow this.

[boxed]

I do try to avoid classes at all costs. I agree that maybe I've gone too far here. What do you think about this @s-kovacevic ?

[fkromer]

@boxed Avoiding classes eases to run code in parallel, is easier to understand and to test. Personally I avoid to use state based classes as well. Of course there are use cases for which classes are made for but pretty sure not for the core functionality of mutmut.

[nklapste]

@boxed Avoiding classes eases to run code in parallel, is easier to understand and to test. Personally I avoid to use state based classes as well. Of course there are use cases for which classes are made for but pretty sure not for the core functionality of mutmut.

Currently, mutmut has two state based classes __main__.Config and __init__.Context however their "methods" are extracted out of the class. While adopting a functional structure can be appealing for parallelism. Having classes that are easy to extend makes more sense if we want to add different mutation behavior, or output formats.

[boxed]

I think we should cross that bridge when we get to it. This is a case of YAGNI. We should worry first about having solid mutations built in, and making mutmut fast and easy to use and generally improving the workflow with dealing with findings.