Move toolkit wrapper-specific tests out of `test_molecule.py`

[jthorton]

Is your feature request related to a problem? Please describe.
In a recent PR #521 we found that tests could be better divided between the test_molecule and test_toolkit files. So I want to start a plan here for how we can refactor the tests to be thorough and logically divided.

Here is @j-wags comment from that PR

In my mind, the toolkit tests in test_molecule should really only be for ensuring that the Molecule-level "wrapper" functions have the right behavior (accepting the right kwargs, calling a ToolkitWrapper or ToolkitRegistry, not mangling exceptions, etc), whereas checking for correctness of toolkit-dependent functionality belongs in test_toolkits. So, actually, I agree with your move here to put the minidrugbank SDF loader in test_toolkit. I just think the minidrugbank MOL2 loader and bad-molecule lists should also be in test_toolkits.

This would say to me that every method of the Molecule class should be tested at least once in the test_molecule file covering different args and fail modes if applicable and ensuring that the correct toolkit method is called. Test_toolkits would then include more thorough testing of the toolkit methods looking for toolkit differences in performance.

[jchodera]

I agree here:

• test_molecule.py should test the Molecule API irrespective of toolkit
• test_toolkit.py should test the ToolkitWrapper/ToolkitRegistry capabilities of all compatible toolkits

[jthorton]

#547 also highlights that we are missing testing of some molecule functions, so we should also have another look at coverage.

[mattwthompson]

not sure this issue should become our master list of things we wish to change in tests, but in #281 I noticed some (existing) tests in test_molecule.py are basically copies of each other between different toolkits, which would ideally be done with pytest.mark.parametrize or some similar magic

[mattwthompson]

Probably not worth the trouble on CI unless GitHub Actions and Travis are nice enough to give us multi-core VMs, but as the test suite slowly grows there is pytest-parallel that works nice locally

$ pip install pytest-parallel # not on conda, although there are other options
$ py.test -v --workers auto # or --workers 4 etc.

[davidlmobley]

@mattwthompson you mentioned this on the list of things requiring feedback; what feedback is needed on this one?

[mattwthompson]

I must have been thinking of something not covered here; this is not blocking any of my current work