Skip to content

Add pytest, enable coverage testing, fix key issue#105

Merged
mvdbeek merged 28 commits intogalaxyproject:masterfrom
rhpvorderman:pytest
Aug 14, 2018
Merged

Add pytest, enable coverage testing, fix key issue#105
mvdbeek merged 28 commits intogalaxyproject:masterfrom
rhpvorderman:pytest

Conversation

@rhpvorderman
Copy link
Copy Markdown
Contributor

Hi. I set out to fix #82 . Then I figured this would be much easier to test using pytest. So I set up pytest as well. Since we use tox, I followed the set of recommendations here:
https://docs.pytest.org/en/latest/goodpractices.html
Also that lead me here:
https://blog.ionelmc.ro/2014/05/25/python-packaging/#the-structure

That is why we now have a src/ folder.
I needed to adapt setup.py for that as well. But it should still work.

I also took the opportunity to enable the code to integrate the coverage results with codacy. But I need @jmchilton to integrate that fully. Alternatively I could set up a galaxyproject organization on codacy, but I am not going to do so without explicit permission from a galaxy core member,and certainly not without bypassing John. That is just not a nice thing to do, given he has set up codacy already on his personal codacy page.

I removed some redundant code from the testing files as well. But with me not being involved from the beginning of this project, this might lead to errors later. So reviews are quite welcome.

@erasche, could you check if this fixes #82? @jmchilton could you have a look as well, given that you release the ephemeris package and I had to change setup.py?

This also adresses #71 and #80 .

Best regards,
Ruben

@galaxyproject galaxyproject deleted a comment Aug 10, 2018
@galaxyproject galaxyproject deleted a comment Aug 10, 2018
@galaxyproject galaxyproject deleted a comment Aug 10, 2018
@galaxyproject galaxyproject deleted a comment Aug 10, 2018
Comment thread src/ephemeris/shed_tools_methods.py Outdated
from bioblend.toolshed import ToolShedInstance


def valid_keys():
Copy link
Copy Markdown
Member

@mvdbeek mvdbeek Aug 10, 2018

Choose a reason for hiding this comment

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

You could also just do

VALID_KEYS = [...]

I think that is a common practice for constants

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

"tool_panel_shed_url",
"install_repository_dependencies",
"install_resolver_dependencies",
"install_tool_dependencies"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

tool_shed_url is also valid, right ? I think I saw that in one of the failing tests

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is a typo/mistake indeed. It should be tool_shed_url. Thanks for pointing this out!

@galaxyproject galaxyproject deleted a comment Aug 10, 2018
@galaxyproject galaxyproject deleted a comment Aug 10, 2018
@galaxyproject galaxyproject deleted a comment from mvdbeek Aug 10, 2018
@galaxyproject galaxyproject deleted a comment from rhpvorderman Aug 10, 2018
@galaxyproject galaxyproject deleted a comment from mvdbeek Aug 10, 2018
@galaxyproject galaxyproject deleted a comment Aug 13, 2018
@galaxyproject galaxyproject deleted a comment Aug 13, 2018
Comment thread tests/test_shed_tools_methods.py Outdated
@@ -0,0 +1,30 @@
#!/usr/bin/env python

from ephemeris.shed_tools_methods import *
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we should avoid * imports as a rule, makes it easier to follow what's being tested.

@galaxyproject galaxyproject deleted a comment Aug 14, 2018
@galaxyproject galaxyproject deleted a comment Aug 14, 2018
@galaxyproject galaxyproject deleted a comment Aug 14, 2018
@galaxyproject galaxyproject deleted a comment Aug 14, 2018
@galaxyproject galaxyproject deleted a comment Aug 14, 2018
@galaxyproject galaxyproject deleted a comment Aug 14, 2018
@galaxyproject galaxyproject deleted a comment Aug 14, 2018
@galaxyproject galaxyproject deleted a comment Aug 14, 2018
@galaxyproject galaxyproject deleted a comment Aug 14, 2018
@galaxyproject galaxyproject deleted a comment Aug 14, 2018
@galaxyproject galaxyproject deleted a comment Aug 14, 2018
@galaxyproject galaxyproject deleted a comment Aug 14, 2018
@galaxyproject galaxyproject deleted a comment Aug 14, 2018
@galaxyproject galaxyproject deleted a comment Aug 14, 2018
@galaxyproject galaxyproject deleted a comment Aug 14, 2018
@galaxyproject galaxyproject deleted a comment Aug 14, 2018
@rhpvorderman
Copy link
Copy Markdown
Contributor Author

I have created a new codacy page: https://app.codacy.com/project/galaxyproject/ephemeris/dashboard
I also created a galaxyproject group for this. @jmchilton is also admin of this group.
On this codacy I disabled the assert warnings. Also coverage is integrated with this codacy page.
The advantage of a more general "galaxyproject" codacy is that more people can control the project.

@rhpvorderman
Copy link
Copy Markdown
Contributor Author

Okay I am done. Made sure the docker container is a fixture so it is automatically deleted after the tests are done.
Coverage is also integrated. I think everything is set up for a completely new test environment. Now we only need to improve coverage by writing more test. But that is something for the future.

@galaxyproject galaxyproject deleted a comment Aug 14, 2018
@galaxyproject galaxyproject deleted a comment Aug 14, 2018
@galaxyproject galaxyproject deleted a comment Aug 14, 2018
@galaxyproject galaxyproject deleted a comment Aug 14, 2018
@galaxyproject galaxyproject deleted a comment Aug 14, 2018
@galaxyproject galaxyproject deleted a comment Aug 14, 2018
Comment thread src/ephemeris/sleep.py
try:
result = result.json()
if options.verbose:
if verbose:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Another direction for the future: the verbose handling should happen via the logging facility. I.e you'd only see INFO and DEBUG levels if you increase verbosity. Which would be nice because it removes the conditional handling here.


# It needs to work well with dev. Alternatively we can pin this to 'master' or another stable branch.
# Preferably a branch that updates with each stable release
GALAXY_IMAGE = "bgruening/galaxy-stable:dev"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That's fine for now, but we can also parameterize the start_container fixture to control which image to start.
Or the simpler variant of controlling it with an ENV var GALAXY_IMAGE = os.environ.get("EPHEMERIS_GALAXY_DOCKER_IMAGE, "bgruening/galaxy-stable:dev")

@mvdbeek mvdbeek merged commit b607c80 into galaxyproject:master Aug 14, 2018
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.

shed-tool install issue with "changeset_revision"

2 participants