Drop in coverage of coordinates module

[utkbansal]

We have a drop in coverage for the coordinates module because my PR #1414 (for porting it to pytest) was merged by mistake.

It is to be noted that the error occurred because of coveralls showing a green signal even when it should have - there was an obvious drop in coverage. Maybe we need to look at how it is configured.

Solution

I will be opening a new PR soon to bring the coverage back to normal.

[jbarnoud]

There is a drop in coverage indeed, but it is very localized. I think we can live with it if the rest of the PR comes soon enough.

For the record, here are the number of tests run by nose and by pytest per file. I ran nose on the commit
c27ace3 that is just before your changes, and I ran pytest on commit 01bd066 that contains your changes. The difference is pytest - nose; a positive would mean that pytest discover more tests than nose, but this is most likely due to my counting method that misses test that have a docstring when processing nose results.

Nose before	Pytest after	Difference	File
254	124	-130	test_gro
81	3	-78	test_xyz
139	71	-68	test_xdr
48	0	-48	test_memory
61	30	-31	test_reader_api
158	156	-2	test_dcd
10	10	+0	test_mmtf
1	1	+0	test_null
11	11	+0	test_crd
11	12	+1	test_pdbqt
120	121	+1	test_dms
121	133	+12	test_pdb
129	130	+1	test_trz
14	15	+1	test_chainreader
144	148	+4	test_trj
147	149	+2	test_timestep_api
153	159	+6	test_dlpoly
16	16	+0	test_mol2
18	18	+0	test_gms
2	2	+0	test_netcdf
24	25	+1	test_pqr
3	3	+0	test_amber_inpcrd
4	4	+0	test_writer_registration
48	48	+0	test_lammps

I get the distributions with the following commands:

# For nose
./mda_nosetests --no-open-files --processes 2 --process-timeout 800 -v coordinates 2>&1 | tee log_nose
cat log_nose | grep -v SKIP | egrep -o 'testsuite\.MDAnalysisTests\.[^ )]+' | cut -f 4 -d . | sort | uniq -c > distrib_nose

# For pytest
pytest -v coordinates/ 2>&1 | tee log_pytest
cat log_pytest | grep -v SKIP | egrep '^coordinates/' | cut -f 1 -d . | cut -f 2 -d / | sort | uniq -c > distrib_pytest

[kain88-de]

There is no need to look much into DCD. We are doing a complete rewrite of it right now anyway.

[kain88-de]

@utkbansal I'd appreciate if you could prioritize this.

[utkbansal]

@kain88-de I'm already working on it. There should be a PR by tomorrow!

[utkbansal]

@kain88-de @richardjgowers @jbarnoud @mnmelo I'm stuck and need help with the test involving reference pattern. The normal convention I've been using so far isn't working for this.
Plus one of the most frustrating things about working with pytest is that it is not showing me the original stack trace, the data that gets logged is almost useless. But I'm guessing this can be configured.

Currently, I'm trying to get it to give me the real stack traces and then I'll try to use fixtures to do this work.

[kain88-de]

put up a PR then it's easier to talk about issues.

[utkbansal]

Okay, I'll open up a PR for auxiliary, it has the exact same convention followed but is smaller and more manageable in comparison to coordinates module and should be easier to review and get feedback. Plus the coordinates module is kind of messy right now.

[utkbansal]

I can't find the reason for the drop in the number of tests collected in test_reader_api. My guess is that it has something to do with yield based tests. pytest counts them as a single test, maybe nose counts them as multiple?!

[kain88-de]

what does the coverage count say?

[utkbansal]

@kain88-de Will check once I'm done with test_xdr

Are we sure that I dont need to work on the dcd tests?

[kain88-de]

@utkbansal yes. I'm doing that in #1372. If you change anything now rebasing will be hell for me.

[kain88-de]

Having the tests for dcd also doesn't matter much since we are rewriting this completely

[utkbansal]

okay, don't want to create a mess for you.

[utkbansal]

@kain88-de Only ran test_reader_api for coverage. There is a 2% drop!!

EDIT:
This might be wrong. Investigating.

[jbarnoud]

Be careful that coverage is not enough of a metric. Multiple tests can cover the same code but test different things.By grepping the verbose output of the tests you should be able to identify the test functions that you may be missing. Le 28 juin 2017 2:40 PM, Utkarsh Bansal <notifications@github.com> a écrit :@kain88-de Only ran test_reader_api for coverage. There is a 2% drop!! —You are receiving this because you were mentioned.Reply to this email directly, view it on GitHub, or mute the thread.