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

[WIP] Add unit tests #122

Merged
merged 11 commits into from
May 19, 2016
Merged

[WIP] Add unit tests #122

merged 11 commits into from
May 19, 2016

Conversation

HubLot
Copy link
Collaborator

@HubLot HubLot commented Apr 11, 2016

Hi,

I'm currently adding unit tests into the package to increase its 'robustness', specially on the different functions of the package (I think the scripts in test_regression.py are very well covered).
I also realized that I introduced some functions before without unit tests associated (booo).

HubLot added 3 commits May 2, 2016 16:41
Add another function to describe the Python ecosystem (version, install
directory).
Update documentation
@HubLot
Copy link
Collaborator Author

HubLot commented May 3, 2016

Hi,

Here a few commits to close this PR:

  • I shrinked the test_data directory (~20Mb -> 2Mb) by shrinking the barstar test files. I removed the water from the gro and xtc and kept only 100 frames of the xtc. I also kept only 3 models of the 2LFU pdb (instead of 10).
  • As discussed in Remove test dir from PyPi package? #121, I created a function called tests() within the root path of PBxplore so the users can run the tests easily. For example :
import pbxplore
pbxplore.tests()

One small issue: usually the test folder is named "tests" and the function is named "test". Here, it's the other way. I could rename those if you want before merging.
Another one is that nose automatically find tests by researching keywords. To prevent it to run this new function tests(), we need to specify the test folder now:

nosetests -v pbxplore/test

I updated those information into the install page of the documentation by creating a chapter called "Testing PBxplore".

  • I create a function named system_info inside the test module which print the version and the install directory of PBxplore dependencies as well as the version of python.

@pierrepo
Copy link
Owner

pierrepo commented May 3, 2016

Great work @HubLot !

One small issue: usually the test folder is named "tests" and the function is named "test". Here, it's the other way. I could rename those if you want before merging.

If you feel this is more mainstream to have the test function named test() and the test directory tests, you can rename them.

Regarding system_info and the module_info() function, be aware that not all Python modules have their version number in __version__.

@jbarnoud what do you think?

@HubLot
Copy link
Collaborator Author

HubLot commented May 4, 2016

Regarding system_info and the module_info() function, be aware that not all Python modules have their version number in version.

Indeed, I'll add a test to prevent the possible error.

@HubLot
Copy link
Collaborator Author

HubLot commented May 12, 2016

I updated the PR.

@jbarnoud can you review?

@jbarnoud
Copy link
Collaborator

I'll look at it on Saturday.

On 12-05-16 16:51, Hub wrote:

I updated the PR.

@jbarnoud https://github.com/jbarnoud can you review?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#122 (comment)

$ python setup.py nosetests


Or within the `PBxplore` package:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does that mean that you must be in the PBxplore directory to tun this command? If not, the sentence is unclear. I would suggest something like:

Or from the `PBxplore` module

@jbarnoud
Copy link
Collaborator

I posted a few comments. Besides these, everything looks good to me.

@HubLot
Copy link
Collaborator Author

HubLot commented May 18, 2016

I updated the PR with @jbarnoud comments.
I also renamed the test folder "tests" and the function 'tests()' into 'test()' to be more 'mainstream' (and of course, updated the documentation and travis).
If you don't agree, I can roll back to the previous version.
If you agree, the PR can be merged :)

@pierrepo pierrepo merged commit 6a0858a into pierrepo:master May 19, 2016
@HubLot HubLot deleted the unit_tests branch May 25, 2016 09:51
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

3 participants