mutmut icon indicating copy to clipboard operation
mutmut copied to clipboard

Make it clearer that mutation happens in-place

Open OddBloke opened this issue 5 years ago • 8 comments

As I have never run mutmut before, I left it running in the background while I continued to do some work, and discovered that the mutations happen on the codebase in-place. I think this is totally reasonable, but it didn't match my assumptions approaching the tool, so I ended up (a) not able to continue working against the codebase, and (b) probably invalidating some of the tests that mutmut performed by writing files while it was running.

It'd be good if this were clearer!

OddBloke avatar May 06 '19 16:05 OddBloke

True. The fact that it's on disk has loads of negative effects (like making parallelization pretty much a non-starter) so that's something I'd like to fix at some point. Do you have some idea of how to make this clear?

boxed avatar May 06 '19 18:05 boxed

Nothing in particular; perhaps just a sentence somewhere in the README/help?

OddBloke avatar May 09 '19 11:05 OddBloke

from: mutmut documentation

To print the results run mutmut results. It will give you output in the form of the commands to apply a mutation:

mutmut apply 3 You can just copy paste those lines and run and you’ll get the mutant on disk. You should REALLY have the file you mutate under source code control and committed before you mutate it!

I've only used mutmut for few minutes, but I'm not entirely sure the above is correct. When I read it, I interpreted it as saying the mutation would be printed on the screen for me to see. What actually happened, if I recall correctly, was nothing was printed on my screen and my source code was changed with no indication that it was being changed. (Win 10). Because there was no indication of anything being done, I randomly tried different numbers. Fortunately, I was able to find and correct all of the changes that were made.

So I have some suggestions.

  1. Perhaps adding a warning similar to:

Running the command "mutmut apply {id}" WILL CHANGE your source code. Use this command wisely.

PS: I tried to use the "<" and ">" around the above "id" but markdown read it as a code marker. I was not sure how to change it, hence this statement.

  1. When the apply command is issued, return a warning / statement to the user that changes were made to the source code. Include file name and line number of the change.

  2. This is a modification of the above suggestion When the apply command is issued, return a statement to the user that changes to the source code (including file name and line number) are about to be made and give the user the option to continue.

pavulon18 avatar Dec 16 '19 13:12 pavulon18

you’ll get the mutant on disk

You read that as "on screen"? It says quite clearly "on disk"!

You also didn't read the warning so why would adding another warning help?

boxed avatar Dec 16 '19 14:12 boxed

But I did read the warning. I might have "misread" the warning, but i did READ the warning so do not tell me that I did not read them.

Also, a second warning, written a different way than the first just might have made me realize that I had misread the first warning.

Just because it is painfully obvious to you what your documentation means does not mean everyone will see it the same way.

After going back to look at it, I still do not believe the documents are as clear as you seem to think they are. For example:

To print the results run mutmut results. It will give you output in the form of the commands to apply a mutation:

mutmut apply 3

The results of "mutmut results" is anything but "in the form of the commands to apply a mutation:"

The results returned by that command are simply a bunch of numbers in specific groupings. They are NOT in the form of the commands to apply a mutation.

pavulon18 avatar Dec 17 '19 05:12 pavulon18

Sorry, I wrote that a bit sleep deprived. I realize now it came off super rude. Again, I'm sorry!

Ok, how about changing mutmut apply to mutmut write? That should be more obvious?

I have updated the outdated readme. I freely admit that I didn't properly read what you wrote :) The relevant section now reads

To print the results run mutmut results. It will give you a list of the mutants grouped by bucket. You can now either show the diff of a mutant with mutmut show 3 or write it to disk with mutmut apply 3.

You should REALLY have the file you mutate under source code control and committed before you mutate it!

boxed avatar Dec 17 '19 07:12 boxed

@boxed , Apology accepted. I really appreciate that. I understand about being sleep deprived. I will also apologize for becoming defensive and retaliatory.

Now on to business. I like your idea about changing mutmut apply to mutmut write. I do think that would help give an additional indication that something could be changed.

I also agree that the changes you made in the documentation are much better than the original.

Although I still like my idea of having the extra statement similar to what I wrote above (although I am biased), I think you've definitely made some improvements in clarity. I am looking at this from the viewpoint of someone who is an inexperienced programmer and who is still being bombarded with a lot of new concepts. There are times when some points of a new concept are not immediately absorbed. I do concede that you do have (and originally had, as well) a statement that should have caught my eye, but I failed to catch it.

I also have a question about mutmut when used in a project using SQLAlchemy. I'll pose my question here but if you think this is not the appropriate place for this, please point me in the right direction.

Should the classes that are used for mapping SQL tables be mutation tested? and if so, how do I go about fixing my classes to kill the mutations? I can supply specific examples of code that I am talking about, if need be.

I have not seen anything online that addresses this issue. These questions could simply be due to my lack of experience.

EDIT: After writing this earlier, I have figured a way to test at least some of the functions in my SQLAlchemy classes that I was confused about. I'll try to build on this to see if I can get most, if not all of them testable.

pavulon18 avatar Dec 17 '19 18:12 pavulon18

Good luck with that! If the mutants aren't really testable, we are discussing more advanced whitelisting ideas in https://github.com/boxed/mutmut/issues/47

boxed avatar Dec 20 '19 14:12 boxed