Run local galaxy via gravity by mvdbeek · Pull Request #1232 · galaxyproject/planemo

mvdbeek · 2022-05-09T09:59:36Z

This drops the run_tests.sh mode completely and leads to a lot of simplification of the code.

planemo/commands/cmd_shed_serve.py

planemo/commands/cmd_shed_test.py

planemo/engine/galaxy.py

planemo/engine/test.py

nsoranzo · 2022-05-25T17:56:21Z

@mvdbeek The unit test failures are legit, I think you mention you already have fixes for them?

mvdbeek · 2022-05-26T08:03:21Z

I'm working on this, yes.

nsoranzo · 2022-05-26T16:55:37Z

Q: with this, we are dropping support for serving/testing on Galaxy pre 22.01, right?

In that case, we should replace unit-nonredundant-noclientbuild-noshed-gx-2105 and unit-nonredundant-noclientbuild-noshed-gx-2109 tox envs with unit-nonredundant-noclientbuild-noshed-gx-2201 in .github/workflows/ci.yaml (and similarly in tox.ini).

mvdbeek · 2022-05-27T09:03:59Z

Q: with this, we are dropping support for serving/testing on Galaxy pre 22.01, right?

No, I will re-introduce the necessary parts. That shouldn't be a big deal, that was just for me to get started.

Co-authored-by: Nicola Soranzo <nicola.soranzo@gmail.com>

If a future / thread is in `time.sleep(1)` it can't be joined.

Galaxy will figure this out (dev wheels on dev), and since we don't use run_tests.sh we shouldn't need any dev wheels.

Previously only worked for test run via run_test.sh, should now work anywhere the option is included.

nsoranzo

Maybe we should pass the timeout to _run() as well, even if not used yet? Feel free to discard if it doesn't make sense!

nsoranzo · 2022-05-31T12:03:40Z

planemo/engine/interface.py

+                job_paths.append(test_case.job_path)
        try:
-            run_response = self._run(runnable, job_path)
+            run_responses = self._run(runnables, job_paths)


Suggested change

run_responses = self._run(runnables, job_paths)

run_responses = self._run(runnables, job_paths, timeout=test_timeout)

nsoranzo · 2022-05-31T12:04:35Z

planemo/engine/interface.py

+        return run_responses

    @abc.abstractmethod
    def _run(self, runnable, job_path):


Suggested change

def _run(self, runnable, job_path):

def _run(self, runnable, job_path, timeout=None):

nsoranzo · 2022-05-31T12:20:43Z

planemo/engine/galaxy.py

-            if self._ctx.verbose:
-                self._ctx.log("Running Galaxy with API configuration [%s]" % config.user_api_config)
-            run_response = execute(self._ctx, config, runnable, job_path, **self._kwds)
+    def _run(self, runnables, job_paths):


Suggested change

def _run(self, runnables, job_paths):

def _run(self, runnables, job_paths, timeout=None):

mvdbeek · 2022-05-31T12:51:44Z

It is used in _run for Galaxy Engines via

planemo/planemo/engine/galaxy.py

Line 49 in 36eb798

run_response = execute(self._ctx, config, runnable, job_path, **self._kwds)

. If we pass the kwds explicitly we'd have to make a copy of the dict so we're not passing the argument twice ... my first version did pass the timeout argument to _run but that didn't look much better to me.

`problem_log` is an optional property that we only fill if we have galaxy logs.

mvdbeek · 2022-05-31T14:58:49Z

It's green!

simonbray · 2022-05-31T16:19:04Z

planemo/galaxy/test/actions.py

-    )
-    action = "Testing tools"
-    return_code = run(ctx, cmd, config.env, action)
-    if return_code != 0 and kwds.get("update_test_data", False):


--update_test_data does not work when I test locally (seems to be no test for it)

Support for this was always partial (no discovered datasets, breaks for contains, from_work_dir etc). How bad would it be to drop this for now and maybe re-implement later?

I think it would be pretty bad to drop it permanently tbh. That option is quite useful.

If it can't go in this PR, I can re-implement it in a new PR (hopefully a more comprehensive version), but I think we shouldn't make a new release without it.

planemo/engine/test.py

planemo/runnable.py

planemo/commands/cmd_shed_test.py

planemo/engine/interface.py

jmchilton · 2022-09-20T16:17:39Z

planemo/galaxy/test/actions.py

-        return_code = run(ctx, cmd, config.env, action)
-
-    _check_test_outputs(xunit_report_file_tracker, structured_report_file_tracker)
-    test_results = test_structures.GalaxyTestResults(


This is no longer used but still in the code base... are we certain the stuff it is doing is being replicated in tool-util? Unfortunately this seems like a question for @jmchilton 😿.

jmchilton · 2022-09-20T16:28:06Z

tests/test_cmd_test.py

                assert (
                    float(tool_test_json["tests"][0]["data"]["time_seconds"]) <= 10
                ), "Test needed more than 10 sec but should time out after 1"
-                assert "Timed out after" in tool_test_json["tests"][0]["data"]["problem_log"], "Time out did not happen"


This scares me - these logs were being cleaned up and merged by code removed? This will cause a degradation in the reports? Again... a question for me...

FWIW we dump that in https://github.com/mvdbeek/galaxy/blob/3aed09379875dc48610fbe86b897d03c6b74b143/lib/galaxy/tool_util/verify/interactor.py#L1271. So workflow tests or tests against external engines have used this for years at this point.

mvdbeek · 2022-09-21T08:49:33Z

I remember now that one of the concerns I had at the time is that I'm not sure we're always correctly shutting down all processes managed by gravity ... unit tests seemed to leave me with lingering processes IIRC.

jmchilton · 2022-09-23T13:36:37Z

Thanks for pushing us forward @mvdbeek - these are really really important changes and simplify things very nicely.

mvdbeek force-pushed the planemo_gravity branch 2 times, most recently from 7d20a94 to 4c2e268 Compare May 10, 2022 17:05

mvdbeek force-pushed the planemo_gravity branch 2 times, most recently from 1e5dd9d to 3eb9ce6 Compare May 24, 2022 11:34

nsoranzo reviewed May 24, 2022

View reviewed changes

nsoranzo mentioned this pull request May 25, 2022

Drop nose galaxyproject/galaxy#13909

Merged

8 tasks

mvdbeek added the wip label May 26, 2022

mvdbeek force-pushed the planemo_gravity branch from 3c57424 to d7ff612 Compare May 26, 2022 14:29

mvdbeek force-pushed the planemo_gravity branch 3 times, most recently from d830e7d to 775000b Compare May 30, 2022 13:18

mvdbeek and others added 12 commits May 30, 2022 15:42

Support starting Galaxy via gravity

130b03a

Use LocalManagedGalaxyEngine instead of run_tests.sh to run tool tests

7651337

Start just one instance to run many test cases

926f64b

Fall back to .venv if no GALAXY_VIRTUAL_ENV defined

3cf6289

Eliminate use of run_tests.sh

04364a1

Use more f-strings

b42183a

Co-authored-by: Nicola Soranzo <nicola.soranzo@gmail.com>

Into start just one

bc53308

Separate test cases based on whether they have a tool_id

a142b28

Maybe fix test shutdown?

e4fcc51

Fix shutdown of tests that use sleep_for_serve

adf853c

If a future / thread is in `time.sleep(1)` it can't be joined.

Stop non-daemon planemo serve during tests

53be41d

Restore support for < 22.01

c27e838

mvdbeek force-pushed the planemo_gravity branch from 5c88a03 to 7aa80ab Compare May 30, 2022 13:43

mvdbeek added 2 commits May 30, 2022 15:54

Drop more unused run_tests.sh functionality

258b148

Drop setup_common_startup_args

a8b1602

Galaxy will figure this out (dev wheels on dev), and since we don't use run_tests.sh we shouldn't need any dev wheels.

mvdbeek mentioned this pull request May 31, 2022

[WIP] Implement serve flag for planemo test #1185

Draft

Fix and extend support for test_timeout flag

36eb798

Previously only worked for test run via run_test.sh, should now work anywhere the option is included.

mvdbeek force-pushed the planemo_gravity branch from 7495bb6 to 36eb798 Compare May 31, 2022 11:20

nsoranzo reviewed May 31, 2022

View reviewed changes

Fix test assertion

140c4bf

`problem_log` is an optional property that we only fill if we have galaxy logs.

mvdbeek force-pushed the planemo_gravity branch from 3691b38 to 140c4bf Compare May 31, 2022 13:33

mvdbeek removed the wip label May 31, 2022

simonbray reviewed May 31, 2022

View reviewed changes

simonbray approved these changes Jun 3, 2022

View reviewed changes

planemo/engine/test.py Outdated Show resolved Hide resolved

planemo/runnable.py Outdated Show resolved Hide resolved

planemo/commands/cmd_shed_test.py Outdated Show resolved Hide resolved

simonbray reviewed Jun 3, 2022

View reviewed changes

planemo/engine/interface.py Outdated Show resolved Hide resolved

mvdbeek added 4 commits June 7, 2022 11:18

Restore update_test_data handling

9bd338b

Adjust signature

2f23cc1

Move install_args_list_to_runnables to runnable_resolve

639fd78

Drop TODO, works now

3e48575

mvdbeek marked this pull request as draft June 8, 2022 15:03

simonbray mentioned this pull request Jun 10, 2022

Add flag to planemo serve to save jobs as test cases #1246

Open

simonbray linked an issue Jun 15, 2022 that may be closed by this pull request

Planemo should not call run_tests.sh #1035

Closed

simonbray mentioned this pull request Jul 7, 2022

Drop support for Python 3.6 #1257

Merged

nsoranzo mentioned this pull request Aug 28, 2022

planemo 0.74.11 fails with Python 3.10: AttributeError: module 'collections' has no attribute 'Callable' #1266

Closed

jmchilton mentioned this pull request Sep 9, 2022

Planemo Roadmap - 2022/2023 #1269

Open

mvdbeek marked this pull request as ready for review September 20, 2022 14:43

nsoranzo requested a review from jmchilton September 20, 2022 15:10

jmchilton reviewed Sep 20, 2022

View reviewed changes

jmchilton merged commit 969e590 into galaxyproject:master Sep 23, 2022

bernt-matthias mentioned this pull request Nov 8, 2022

Planemo test is ignoring --failed #1316

Open

	run_responses = self._run(runnables, job_paths)
	run_responses = self._run(runnables, job_paths, timeout=test_timeout)

	def _run(self, runnable, job_path):
	def _run(self, runnable, job_path, timeout=None):

	def _run(self, runnables, job_paths):
	def _run(self, runnables, job_paths, timeout=None):

Conversation

mvdbeek commented May 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nsoranzo commented May 25, 2022

Uh oh!

mvdbeek commented May 26, 2022

Uh oh!

nsoranzo commented May 26, 2022

Uh oh!

mvdbeek commented May 27, 2022

Uh oh!

nsoranzo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mvdbeek commented May 31, 2022

Uh oh!

mvdbeek commented May 31, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mvdbeek commented Sep 21, 2022

Uh oh!

jmchilton commented Sep 23, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mvdbeek commented May 9, 2022 •

edited

Loading