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

[Experimental] Initial attempt at adding JUnit XML support #55

Merged
merged 8 commits into from
Dec 21, 2018

Conversation

jmdacruz
Copy link
Contributor

@jmdacruz jmdacruz commented Dec 19, 2018

Related to #49. Most CI tools include some support for interpreting JUnit XML output, extracting information about total executed tests vs. passed, capturing stdout/stderr from test execution, etc. This is an attempt at implementing some very rudimentary support for JUnit XML in mutmut.

In order to get the report in this new format, simply run mutmut junitxml (similar to running mutmut results)

Notes for discussion:

  • This could be exposed as a modified/parameter to the results command, instead of a new command (it's just a different way of reporting the results).
  • What to show on the stdout and stderr attributes. Right now I'm simply adding Mutant.line.line, but ideally we want to show the diff of the mutant. It seems the code for generating the diff is on the __main__.py file, so maybe that can be refactored and reused there?
  • What is a failure vs. error vs skipped? I'm using failure for surviving mutants, error for timeouts, and skipped for both untested and suspicious. A flag could be added to determine if suspicious mutants should be skipped or considered failures/errors

@codecov-io
Copy link

codecov-io commented Dec 19, 2018

Codecov Report

Merging #55 into master will increase coverage by 0.86%.
The diff coverage is 79.54%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #55      +/-   ##
==========================================
+ Coverage   82.44%   83.31%   +0.86%     
==========================================
  Files           3        3              
  Lines         735      767      +32     
==========================================
+ Hits          606      639      +33     
+ Misses        129      128       -1
Impacted Files Coverage Δ
mutmut/cache.py 83.64% <78.37%> (-1.19%) ⬇️
mutmut/__main__.py 73.33% <85.71%> (+2.37%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 192de1e...d5d55ad. Read the comment docs.

def print_result_cache_junitxml():
test_cases = []
l = list(select(x for x in Mutant))
for filename, mutants in groupby(l, key=lambda x: x.line.sourcefile.filename):
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need the groupby here, unless we want to create separate test suites.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually we do, so that mutants are listed on the report by file (even if they are not really grouped)

@jmdacruz
Copy link
Contributor Author

jmdacruz commented Dec 19, 2018

This is what the rendered JUnit XML output looks like with a tool such as xunit-viewer (get it here). Install it and run it like xunit-viewer --port=8080 --results=output.xml, where output.xml is the result of doing mutmut junitxml > output.xml:

image

And this is what a suspicious mutant looks like (see that the unified diff of the mutant is included on the report):

image

@@ -136,6 +139,51 @@ def print_stuff(title, query):
print_stuff('Untested', select(x for x in Mutant if x.status == UNTESTED))


def get_unified_diff(argument, dict_synonyms):
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactoring the code that was performing the unified diff on __main__.py, and bringing it here so that we can also use it for the JUnit report

@boxed
Copy link
Owner

boxed commented Dec 21, 2018

I like it! Fairly small diff and seems pretty feature complete. The new command seems reasonable to me. As for the status of the suspicious mutants, I'm fine with having them just not be in the output.

This change seems pretty ready to merge as is I think. What do you think?

@jmdacruz
Copy link
Contributor Author

Glad you liked it! Let me add an argument to the command in order to manipulate what to do with the suspicious mutants, and then we can merge, sounds good?

@boxed
Copy link
Owner

boxed commented Dec 21, 2018

Great!

@jmdacruz
Copy link
Contributor Author

jmdacruz commented Dec 21, 2018

I added a couple of parameters to change the policy for dealing with suspicious and untested mutants. A few notes for future reference:

  • I don't think I would be able to maintain the patch coverage target without changing the way the tests are parameterized, since the execution of the foo.py test does not generate suspicious mutants. The only alternative would be to just test the junit report function by mocking the Mutant DAO object. Let me know if you want me to do that now.
  • I added some documentation on the README.rst file. I noticed that there is improper use of backticks on the document (those work on markdown files, but not on RST), so I used them too on purpose so that this can fix in a separate PR (either use proper RST or move to markdown altogether)
  • I added the arguments for the report as global options in click, but ideally we should add support for click's subcommand feature: http://click.palletsprojects.com/en/7.x/commands/. This provides greater control on arguments for each individual command. I'll open a separate issue to discuss this (the change would be backwards compatible)

That's it, ready to merge (I suggest "squash and merge", since there is a lot of noise on this PR)!

@boxed
Copy link
Owner

boxed commented Dec 21, 2018

Yea the codecov thing is bs and should be ignored. I should google on how to turn that off sometime :P

I use RST for the documentation generation with Sphinx. You could have fixed it in this PR that's fine. I'm not a Puritan for tiny stuff like that.

@boxed boxed merged commit b9db8dd into boxed:master Dec 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants