Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Applied mutation causes syntax error #52

Closed
konrad-kocik opened this Issue Dec 16, 2018 · 13 comments

Comments

Projects
None yet
4 participants
@konrad-kocik
Copy link

konrad-kocik commented Dec 16, 2018

Hi,

mutmut version: 1.1.0

I'm using Python 3.7 so in my code base I'm using this syntax:
def except_to_bool(_func=None, *, exc=Exception, to=False):

When I apply mutmut it generates this mutant:
def except_to_bool(_func=None, /, exc=Exception, to=False):
which should not be generated as it produces syntax error.

If mutant should be generated here I imagine it should look like this:
def except_to_bool(_func=None, exc=Exception, to=False):
(simply remove star that enforces keyword arguments)

I even have test for this code (which is checking whether it is not possible to pass positional arguments to this function), but mutant still survives it. I think this is because mutant causes syntax error.

Either fix mutant generation with star, that enforces keyword arguments, or skip generating mutant at all in such case, please :)

@boxed

This comment has been minimized.

Copy link
Owner

boxed commented Dec 16, 2018

Well I agree this mutant is bad but if your test suite survives a syntax error something else is wrong here. If you apply this mutant to your source code your test suite should fail. Does it?

@konrad-kocik

This comment has been minimized.

Copy link
Author

konrad-kocik commented Dec 16, 2018

My tests fail if I apply this mutant manually and run my suite directly from pytest:

E       def except_to_bool(_func=None, /, exc=Exception, to=False):
E                                      ^
E   SyntaxError: invalid syntax

but they do not fail if mutant is applied by mutmut and suit is run by it:

(exceptbool) xxx@xxx:~/projects/exceptbool$ mutmut run

- Mutation testing starting - 

These are the steps:
1. A full test suite run will be made to make sure we 
   can run the tests successfully and we know how long 
   it takes (to detect infinite loops for example)
2. Mutants will be generated and checked

Mutants are written to the cache in the .mutmut-cache 
directory. Print found mutants with `mutmut results`.

Legend for output:
🎉 Killed mutants. The goal is for everything to end up in this bucket. 
⏰ Timeout. Test suite took 10 times as long as the baseline so were killed.  
🤔 Suspicious. Tests took a long time, but not long enough to be fatal. 
🙁 Survived. This means your tests needs to be expanded. 

1. Using cached time for baseline tests, to run baseline again delete the cache file

2. Checking mutants
⠇ 6/6  🎉 5  ⏰ 0  🤔 0  🙁 1
@boxed

This comment has been minimized.

Copy link
Owner

boxed commented Dec 16, 2018

That is strange. Can you supply the code?

@konrad-kocik

This comment has been minimized.

Copy link
Author

konrad-kocik commented Dec 16, 2018

https://github.com/konrad-kocik/exceptbool

Tested method can be found here: exceptbool/exceptbool.py -> except_to_bool
Tests can be found here: tests/test_execeptbool.py
Setup your virtualenv with: pip install -r requirements_dev.txt

@boxed

This comment has been minimized.

Copy link
Owner

boxed commented Dec 17, 2018

I agree this is a bug, but it's a performance issue mostly. When I run mutmut on exceptbool I get just one surviving mutant like you get but it's this one:

>>mutmut show 3
--- exceptbool/exceptbool.py
+++ exceptbool/exceptbool.py
@@ -18,7 +18,7 @@
     :type: bool
     """
     def decorator(func):
-        @wraps(func)
+
         def wrapper(*args, **kwargs):
             try:
                 func(*args, **kwargs)

You're applying a mutant that was killed:

> mutmut show 1
--- exceptbool/exceptbool.py
+++ exceptbool/exceptbool.py
@@ -1,7 +1,7 @@
 from functools import wraps
 
 
-def except_to_bool(_func=None, *, exc=Exception, to=False):
+def except_to_bool(_func=None, /, exc=Exception, to=False):
     """
     Makes decorated function return bool instead of raising an exception by
     converting given exception(s) into given bool value.

If you run just this mutant with mutmut run 1 it will report this as killed.

@konrad-kocik

This comment has been minimized.

Copy link
Author

konrad-kocik commented Dec 17, 2018

@boxed

This comment has been minimized.

Copy link
Owner

boxed commented Dec 17, 2018

They will not ;)

@konrad-kocik

This comment has been minimized.

Copy link
Author

konrad-kocik commented Dec 17, 2018

Well, you were right, of course :) I needed to extend my test suite to kill that mutant. But this is exactly why I wanted to perform mutation tests in the first place! So thank you for that.

Final question: will you still try to solve this problem with transforming * into / in context of keyword arguments enforcement? Or will it stay that way?

@boxed

This comment has been minimized.

Copy link
Owner

boxed commented Dec 17, 2018

It should be fixed yes. It's a performance issue (albeit a small one) if nothing else.

boxed added a commit that referenced this issue Jan 7, 2019

boxed added a commit that referenced this issue Jan 8, 2019

@nedbat

This comment has been minimized.

Copy link

nedbat commented Feb 24, 2019

BTW, with version 1.3.1, I have a survived mutant like this:

$ mutmut show
To apply a mutant on disk:
    mutmut apply <id>

To show a mutant:
    mutmut show <id>


Survived 🙁 (9)

---- src/templite.py (8) ----

10, 27, 35, 43, 44, 56, 106, 150

---- src/__init__.py (1) ----

153

$ mutmut show 153
--- src/__init__.py
+++ src/__init__.py
@@ -1,2 +1,2 @@
-from .templite import *
+from .templite import /

I've laid out my package wrong, which is why this survives the test suite, but it still is not a valid mutation.

@boxed

This comment has been minimized.

Copy link
Owner

boxed commented Feb 25, 2019

Maybe I should have a special pass for finding files that can survive syntax errors because this stuff keeps happening :P

Yea, that should definitely be fixed.

@nklapste

This comment has been minimized.

Copy link
Collaborator

nklapste commented Feb 26, 2019

Maybe we should skip mutating any import lines.

Is there a way to detect lines defining import and from foo import bar statements within a parso.python.tree. If so we could add a rule to skip mutating these lines. I see no real value in messing up imports for mutation testing, it seems much too high level error injection to validate any real unit tests.

@boxed

This comment has been minimized.

Copy link
Owner

boxed commented Mar 1, 2019

True, imports that are broken are for static analysis tools to handle. I'll fix it.

@boxed boxed closed this in 662762d Mar 1, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.