Option to test tools on update/install for Galaxy 18.05.#81
Option to test tools on update/install for Galaxy 18.05.#81jmchilton merged 1 commit intogalaxyproject:masterfrom
Conversation
|
@jmchilton can you rebase on master? The commits from #78 are now in. |
| "--test", | ||
| action="store_true", | ||
| dest="test", | ||
| help="Run tool tests on install tools, requires Galaxy 18.05 or newer." |
There was a problem hiding this comment.
This requirement should be hardcoded somewhere
gi.config.get_config()["version_major"] returns the version.
There was a problem hiding this comment.
I don't know about this - Nate had mentioned wanting to get this on usegalaxy.org and we were intending to maybe place the Galaxy-side APIs in the usegalaxy branch to allow this testing against our variant of 18.01. I guess we could just hack up ephemeris though if we do get around to setting this up. I'll try to find a spot to make this change.
| "--test_json", | ||
| dest="test_json", | ||
| help="If --test is specified, record tool test output to specified file. " | ||
| "This file can be turned into nice reports with ``planemo test_reports <output.json>``." |
There was a problem hiding this comment.
Personal taste: I would leave out the word 'nice' here.
| help="If --test is specified, also run tool tests on repositories already installed " | ||
| "(i.e. skipped repositories)." | ||
| ) | ||
|
|
| for tool in installed_tools: | ||
| galaxy_interactor_kwds = { | ||
| "galaxy_url": re.sub('/api', '', self.gi.url), | ||
| "master_api_key": self.gi.key, |
There was a problem hiding this comment.
Will self.gi.key always return the key? Also if initiated with email and password. I tested it and it seems to be the case. But is this certain?
There was a problem hiding this comment.
Yes - as long as login_required=True with get_galaxy_connection - which it is for this command. It'd be nice if galaxy-lib had a variant of the client code that consume and used a bioblend GalaxyInstance but it doesn't yet so I think this is okay for now? I will admit this is a hack though - let me know if you need me to push the hack down a layer into galaxy-lib.
| log.info("All repositories have been processed.") | ||
| log.info("All repositories have been installed.") | ||
| if self.test: | ||
| installed_tools = [] |
There was a problem hiding this comment.
This should get its own method? test_repositories. It is a lot of extra code and it has its own specific function.
There was a problem hiding this comment.
Agreed - I'll try to make this change.
| test_results = [] | ||
|
|
||
| for tool in installed_tools: | ||
| galaxy_interactor_kwds = { |
There was a problem hiding this comment.
This whole code block can be in a test_tool method. This makes it a lot cleaner:
for tool in installed_tools:
test_information=self.test_tool(tool)
# Some code to gather all the test information
# Some code to write al the test information to a jsonThere was a problem hiding this comment.
Agreed - I'll try to make this change.
|
I am all for integrating the testing within the installation method. But this causes indeed the exit code problem. Doing two things at the same time creates ambiguity. |
Yes - I think I agree. I'll see if I can make the exit code reflect the testing status.
Absolutely - this is an excellent idea - I'll try to reorganize this code to do that. |
jmchilton
changed the title
2 participants
Builds on #78 from @rhpvorderman since it modifies similar files.
While it does run the tests and produce a JSON output file that should be usable with
planemo test_reports(http://planemo.readthedocs.io/en/latest/commands.html#test-reports-command) after the next Planemo release, marked as WIP because:shed-toolsexit code.Do we want to do all the installs and then all the tests like this or should we try to interleave them or should we test as we install or should be make this configurable?Lets just stick with doing them at the end and leave things be for now - we can revisit in the future.Also, what of these questions can we delay to a second iteration?
Implements #76.