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.

Sign up for GitHub

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

Jump to bottom

Coverage #1328

Merged
merged 5 commits into from
Jan 27, 2012
Merged

Coverage #1328

Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
426f233
Produce coverage xml reports from subprocess test runners.
takluyver Jan 25, 2012
7ef51b8
Use standard command for coverage.
takluyver Jan 25, 2012
ebe0acc
Use separate option --with-xml-coverage to ask for coverage xml output.
takluyver Jan 26, 2012
b2b7f88
Don't raise errors from coverage post-processing.
takluyver Jan 26, 2012
6996a39
Use explicit for loop to find section in testing.
takluyver Jan 27, 2012
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 13 additions & 2 deletions IPython/testing/iptest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,8 @@ class IPTester(object):
call_args = None
#: list, process ids of subprocesses we start (for cleanup)
pids = None
#: str, coverage xml output file
coverage_xml = None

def __init__(self, runner='iptest', params=None):
"""Create new test runner."""
Expand All @@ -285,9 +287,14 @@ def __init__(self, runner='iptest', params=None):
# Assemble call
self.call_args = self.runner+self.params

sect = [p for p in self.params if p.startswith('IPython')][0]
Copy link
Member

Choose a reason for hiding this comment

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

Why build a full listcomp here only to keep its first term? This is equivalent and I also think clearer:

for sect in self.params:
  if sect.startswith('IPython'): break

Copy link
Member Author

Choose a reason for hiding this comment

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

I find the listcomp marginally clearer than assignment by for loop, but I'm happy to change it.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I guess it's also a little obscure, but in a different way. I had to reread the listcomp to realize what was going on, and I tend to frown upon constructing a whole data container just to pull back its first element out. Can you think of a third, clearer solution?

If not, I'll leave the final decision up to you, and I'd be OK if you keep the listcomp. Your call.

Copy link
Member

Choose a reason for hiding this comment

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

Actually there may be one meaningful difference: if there are no matches, the listcomp will raise IndexError while the for loop could handle it more cleanly with an else clause (the listcomp would need a try/except).

Just a thought...

if '--with-xunit' in self.call_args:
sect = [p for p in self.params if p.startswith('IPython')][0]
self.call_args.append('--xunit-file=%s' % path.abspath(sect+'.xunit.xml'))

if '--with-xml-coverage' in self.call_args:
self.coverage_xml = path.abspath(sect+".coverage.xml")
self.call_args.remove('--with-xml-coverage')
self.call_args = ["coverage", "run", "--source="+sect] + self.call_args[1:]

# Store pids of anything we start to clean up on deletion, if possible
# (on posix only, since win32 has no os.kill)
Expand Down Expand Up @@ -319,11 +326,15 @@ def _run_cmd(self):
def run(self):
"""Run the stored commands"""
try:
return self._run_cmd()
retcode = self._run_cmd()
except:
import traceback
traceback.print_exc()
return 1 # signal failure

if self.coverage_xml:
subprocess.call(["coverage", "xml", "-o", self.coverage_xml])
return retcode

def __del__(self):
"""Cleanup on exit by killing any leftover processes."""
Expand Down