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

Coverage #1328

Merged
merged 5 commits into from Jan 27, 2012
Merged

Coverage #1328

merged 5 commits into from Jan 27, 2012

Conversation

takluyver
Copy link
Member

Another branch of playing with the tests, this time for coverage reports.

The output looks like this: https://jenkins.shiningpanda.com/ipython/job/ipython-xunit-test/6/cobertura/

@ivanov
Copy link
Member

ivanov commented Jan 25, 2012

am I supposed to be able to just run this with just iptest --with-coverage? I'm getting

Traceback (most recent call last):
  File ".../IPython/testing/iptest.py", line 329, in run
    retcode = self._run_cmd()
  File ".../IPython/testing/iptest.py", line 317, in _run_cmd
    subp = subprocess.Popen(self.call_args)
  File "/usr/lib/python2.6/subprocess.py", line 633, in __init__
    errread, errwrite)
  File "/usr/lib/python2.6/subprocess.py", line 1139, in _execute_child
    raise child_exception
OSError: [Errno 2] No such file or directory

for every test group, followed by this output:

----------------------------------------
Runner failed: IPython.core
You may wish to rerun this one individually, with:
coverage run --source=../IPython/testing/iptest.py IPython.core

also, shooting from the hip - do you want to make it easy to clean up these xml files that will be created?

@takluyver
Copy link
Member Author

It's mainly for ShiningPanda. The script there simply does rm -f *.coverage.xml to clear them up.

Note that you need coverage installed for this to run. If you're on Ubuntu/Debian like me, you need to install it from PyPI, because the packaged version is run as python-coverage (or check out bad4947, which has the Debian command from my local testing).

@fperez
Copy link
Member

fperez commented Jan 26, 2012

Mmh, Shining Panda appears to be busted right now, so I can't look at the results. I keep getting redirected to their homepage every time I try to go to our Jenkins url.

BTW, any particular reason you made this branch in origin and not in takluyver? Normally we make branches in the main repo only when we need to all have write access to them, so typically just for long, complex problems. If it was an accident, don't bother cleaning it up: I'm sure we'll want this merged, so we can just delete the branch from the repo when we're done.

The code looks good, but I suggest one change: let's make the option be called --with-xml-coverage. Otherwise, this would shadow the already valid nose --with-coverage option, which can be used to provide quick and dirty console-based coverage reports.

@takluyver
Copy link
Member Author

Hmm, I could simply point ShiningPanda at my own repo. I'd had a vague feeling it wanted to pull everything from a single IPython repo, but there's no truth in that. If I do any more of these branches, I'll put them in my own repo.

Good point, I'll call it --with-xml-coverage (although my testing suggested --with-coverage- doesn't actually work in our test architecture - it says there's 0 coverage of all IPython files).

@fperez
Copy link
Member

fperez commented Jan 26, 2012

On Thu, Jan 26, 2012 at 2:49 AM, Thomas
reply@reply.github.com
wrote:

Hmm, I could simply point ShiningPanda at my own repo. I'd had a vague feeling it wanted to pull everything from a single IPython repo, but there's no truth in that. If I do any more of these branches, I'll put them in my own repo.

Ah no worries! Now I understand, it was to make it simpler to pick up
from SP. That's no problem, I just wasn't sure if it had been an
accidental overlook.

Good point, I'll call it --with-xml-coverage (although my testing suggested --with-coverage- doesn't actually work in our test architecture - it says there's 0 coverage of all IPython files).

On my box, it did spew out long plaintext reports, don't know why the
difference.

@takluyver
Copy link
Member Author

I changed --with-coverage to --with-xml-coverage. Helpfully, testing it on ShiningPanda revealed that they've made a change to their build system (so I'm going to add rm -rf build/ to all the test scripts). Happily, after a bit of cussing, we're back to working coverage reports: https://jenkins.shiningpanda.com/ipython/job/ipython-xunit-test/14/cobertura/?

It looks like we no longer need to reinstall all our dependencies on every test run, which might help with getting more complex dependencies installed. But I don't know if we can realistically compile numpy. Perhaps I can ask them to install the Debian package for it.

@fperez
Copy link
Member

fperez commented Jan 27, 2012

Mmh, for some reason this one picked up a conflict, could you give it a quick rebase?

@takluyver
Copy link
Member Author

All done. The conflict was just from the debugging line I'd left in the other day, which you removed.

@@ -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...

@fperez
Copy link
Member

fperez commented Jan 27, 2012

Other than my tiny comment, this looks good to go!

@takluyver
Copy link
Member Author

I've used an explicit for loop, but if there's no match, I'm just raising an error (because as far as I know, there should always be a match at that point).

@fperez
Copy link
Member

fperez commented Jan 27, 2012

Looks good, merging now. Thanks!

fperez added a commit that referenced this pull request Jan 27, 2012
Add XML coverage support to the test suite with new --with-xml-coverage flag.  The resulting XML output files are understood by Jenkins, so now our automated test suite will have coverage reports.
@fperez fperez merged commit e0143f7 into master Jan 27, 2012
@takluyver
Copy link
Member Author

Thanks, Fernando. I'll set up ShiningPanda to use this.

@fperez
Copy link
Member

fperez commented Jan 27, 2012

On Fri, Jan 27, 2012 at 3:32 AM, Thomas
reply@reply.github.com
wrote:

Thanks, Fernando. I'll set up ShiningPanda to use this.

Great! With that in place, we can begin to focus on problem areas
with especially low coverage. I'm sure we'll find (and fix!) tons of
issues in the process.

@Carreau
Copy link
Member

Carreau commented Jan 27, 2012

@takluyver is it possible to add your own 'custom' VM to shining panda ?
If so I can try to have a virtualbox instance running on Week-ends while I'm not using my computer lab if it helps.

@takluyver
Copy link
Member Author

I don't think it can be integrated with ShiningPanda (yet, at least), but if you want to run the tests, you can set up your own Jenkins instance: http://jenkins-ci.org/

What's available on your machines? The biggest gap at the moment is Windows testing. Testing with packages like Numpy installed would also be valuable, but we may be able to ask ShiningPanda to install that for us.

@Carreau
Copy link
Member

Carreau commented Jan 27, 2012

As for now I've a virtual box machine with linux Mint Debian version, a few hours old, but I could easily add anything else and launch it regularly with a cron and VboxManage. I hope beeing able to install Mac OS X Lion in one VM, but so far I've not been able to.
The issues with windows testing is liscence and remote control, and I don't want to/don't had enough time to set this up.

The physical computer is an iMac core i7 2.8 Ghz [8 cores] with 8 Gb of ram, so it could be easy to load 1 or 2 VMs during the 'night' (depending of where you are on earth) and week-end (this is more constant across time zones...).

So if you have any vbox image you would like me to run ... it would just not be availlable 24/7.
Only catch is that's it's behind a (not so well configured-) proxy, so it might take some time to get the setup right for aptitude/git/curl...

@takluyver
Copy link
Member Author

That's fair enough, I didn't think you were likely to want to set up a Windows server.

Actually, if the physical machine is an iMac, would it be possible to just use a virtualenv on that, rather than a whole separate virtual machine? Testing on OS X gives us a wider base than another Debian-based Linux system. If not, we can still work something out - if we can get the Debian packaged version of Numpy installed on ShiningPanda, maybe your machine could test against the latest released versions Numpy/Matplotlib.

Weekly testing would be fine. If you want to do nightly testing, I've got ShiningPanda set up to run at 9am UK time/10 am Western Europe/1am California, which I think should avoid most of our prime hacking time, at least until we get Chinese contributors.

@fperez
Copy link
Member

fperez commented Jan 27, 2012

On Fri, Jan 27, 2012 at 1:36 PM, Thomas
reply@reply.github.com
wrote:

Testing on OS X gives us a wider base than another Debian-based Linux system.

An osx shining panda build would be awesome to have!

@Carreau
Copy link
Member

Carreau commented Jan 30, 2012

I have virtualenv working, should I try to get jenkins working or running iptest/something else is sufficient ?

@takluyver
Copy link
Member Author

Whatever you've got time for. Jenkins will automate running the tests regularly, and presenting the output as HTML, but anything that runs iptest and flags up failures is good. I've never tried to install Jenkins, so I don't know how complex it is.

For reference, this is the script that runs on ShiningPanda (the environment variables shouldn't be necessary if you can install ZMQ systemwide):

export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:/home/slave/local/lib
export ZMQ_DIR=/home/slave/local
pip install nose coverage
pip install pyzmq
pip install readline

rm -rf build/
python setup.py install

cd env
rm -f *.xunit.xml
rm -f *.coverage.xml
if [ -n "$(which iptest3)" ]
  then
    iptest3 --with-xml-coverage --with-xunit
  else
    iptest --with-xml-coverage --with-xunit
fi

@Carreau
Copy link
Member

Carreau commented Feb 1, 2012

Is there any way to run iptest directly on the dev version without having to install it ?
Moreover, iptest is finding my other ipython repo (~/ipython instead of ~/env/ipython), the one where I work, and made change... can I change that ?

otherwise : zip of ipset with coverage on master should I also include stdout ?

@takluyver
Copy link
Member Author

I don't think there's a way to run it without installing it, but installing it in a virtualenv should work (that's how I develop and test). With the virtualenv active, iptest should refer to the IPython installed inside that virtualenv.

@ivanov
Copy link
Member

ivanov commented Feb 2, 2012

Is there any way to run iptest directly on the dev version without having to install it ?

if you install the dev version using ipython setup.py develop -
then you'll always be running whatever IPython is in your
git directory.

@Carreau
Copy link
Member

Carreau commented Feb 2, 2012

Thanks, it almost works, I still have to refer to the env/bin/ipytest in my env manually, might have done something wrong... somewhere.
won't have time to but i'll try to wrap everything in a cron soon.

@Carreau
Copy link
Member

Carreau commented Feb 3, 2012

ok, my iMac should update / run test of ipython then upload all the xmls files here in a zip named after git-describe with the redirection of stdout/stder of installation in .log files, every few hour at night.

It's getting late now, so I'm going home and will see on monday. If you need me to change smth of want a different structure to get the xmls files let me know.

Matthias

@takluyver
Copy link
Member Author

Excellent, thanks @Carreau . Now I just need to work out how to view the XML files outside of Jenkins.

@Carreau
Copy link
Member

Carreau commented Feb 3, 2012

@takluyver Can't you just make a dumb ShiningPanda script that download the zip and expand it making Jenkins think that it did run the test ?

@takluyver
Copy link
Member Author

I like it. Can you set up your test script so that after each run it makes a symlink to something like test-latest.zip?

@Carreau
Copy link
Member

Carreau commented Feb 3, 2012

That I can do from home, and it's done ... it should work automatically

@takluyver
Copy link
Member Author

@takluyver
Copy link
Member Author

It's set up to grab the results at 9am French time every day. Let me know
if I should adjust that to match when your tests finish.

@Carreau
Copy link
Member

Carreau commented Feb 3, 2012

You're missing the grave accent on the a of "voilà", but it is nice.
9am is ok, It will have plenty of time to build durring the night.
I'll also change the cron to build more often on week-end.

@takluyver
Copy link
Member Author

Ah, thanks for the tip. Let me know what extra times to set it to grab
results at the weekend.

@fperez
Copy link
Member

fperez commented Feb 4, 2012

On Fri, Feb 3, 2012 at 10:35 AM, Thomas
reply@reply.github.com
wrote:

Et voila: https://jenkins.shiningpanda.com/ipython/job/ipython-mac/

Guys, many thanks for setting this up, it's great!!

mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
Add XML coverage support to the test suite with new --with-xml-coverage flag.  The resulting XML output files are understood by Jenkins, so now our automated test suite will have coverage reports.
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

4 participants