Tests should make MDAnalysis raise exceptions

[jbarnoud]

Looking at #592, and where the exclamation marks in formatted string may be an issue, I realized we rarely test the cases where exceptions are raised. These parts account for a significant part of the uncovered code. While it could seem harmless, it means we are not sure the expected exception is raised for a given error. Also, we are not sure the code to raise the exception is correct as it is the case in #592.

Tests should account for bad inputs and case MDAnalysis to raise its exceptions.

[orbeckst]

Good idea. However, it's a pretty big task and although not very difficult, a bit daunting. It would be better to split it into more well defined tasks such as "tests for lib.transformations should check exception handling". This will (hopefully) make it more likely that someone picks up an issue (and it would also make it easier for new contributors to contribute).

New sub issues should reference this issue as a "parent" issue.

[richardjgowers]

Yep, definitely something that needs doing when writing tests.

The guide on writing tests probably needs to get spun out into its own page (separate from details on running them). And then an example of how both good & bad usage should be tested.

[orbeckst]

Anyone applying for GSoC and looking for a fixable issue should look at this one. Follow @fiona-naughton 's lead and identify one module or even just one class where we don't test an exception:

1. go to the coverage report
2. click on the build number and
3. under FILES, click on the All tab
4. pick a file name and click on it
5. search for red lines with except and identify file and class/function

Then find the test file under testsuite/MDAnalysisTests that tests the code that you identified.

1. Raise an issue in the tracker for the specific exceptions that you want to test (and mention this issue Tests should make MDAnalysis raise exceptions #597)
2. Add a test or tests that raise the exception.
3. Submit a PR (referencing your new issue).

[CalebMadsen]

Hey there,

I really appreciate you spelling things out for me. I went to the coverage report and just clicked the top build number (1700). I clicked through a few files and found this cone : https://coveralls.io/builds/5512450/source?filename=miniconda%2Fenvs%2Fpyenv%2Flib%2Fpython2.7%2Fsite-packages%2FMDAnalysis%2Fcore%2F__init__.py .
At about the 218th line there is an Except RaiseError. Is this what I'm looking for? If so the Class says Flag(object). I am not sure I understand how to use the tests. I tried this in Python:

importMDAnalysis.tests
MDAnalysis.tests.test()

And it returns a failure. I'm not sure what MDAnalysis.tests.test() does though. If you have any advice I 'd appreciate it. Thanks.

[jbarnoud]

This is indeed the kind of lines we would like the tests to cover. Ideally, we would like you to choose a file and have the tests cover all the exceptions it can raise. The file you chose is a good candidate, yet we plan to remove the flag feature (#782), so it may not be the most useful to tackle.

Explore the wiki. It has a full page on how to run and write tests.

[vedantrathore]

@kain88-de @jbarnoud I would like to work on this, could anyone give me a pointer on how to start with this?

[jbarnoud]

This issue is all about increasing the test coverage. Too often we write tests to make sure what should work does, but we omit the tests that make sure the case that should fail do fail as we expect. You can find the test coverage on our coverall page: <https://coveralls.io/github/MDAnalysis/mdanalysis> There is a project on github that tries to follow what is being done to reach 90% coverage (the project does not seem super up to date, though): <https://github.com/MDAnalysis/mdanalysis/projects/2> Anyway, the wiki guide for developers is a good read: <https://github.com/MDAnalysis/mdanalysis/wiki/Guide-for-Developers>. In your case, the page about writing tests is the one not to miss: <https://github.com/MDAnalysis/mdanalysis/wiki/UnitTests>. Hopefully it is up to date... Have fun!

…

On 15/01/17 00:26, Vedant Rathore wrote: @kain88-de <https://github.com/kain88-de> @jbarnoud <https://github.com/jbarnoud> I would like to work on this, could anyone give me a pointer on how to start with this? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#597 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABUWuj47bga8MOzW2DL4ZSRxlUTC9F3xks5rSVmjgaJpZM4G2Yjr>.