Tool Shed 2.0 fixes

[nsoranzo]

• toolshed GitHub workflow fixes:
  • Remove duplicated cache key that prevented the tests from being run
  • Expand matrix to test all combinations on Python 3.7 and 3.11
  • Install node from setup-node action
• Fix issues reported by yarn build
• Explicitly add playwright to dev dependencies
• Restore GET /api/categories/<encoded_id> and the deleted field in the response.
• Add some type annotations.
• Remove unused functions and re-exports.
• Misc bug fixes.

A bunch of ToolShed tests are failing, but they were simply not being run previously.
We can either merge this and fix the failing tests in separate PRs, or use this PR to push the fixes, but I don't have time to look into them currently.

How to test the changes?

(Select all options that apply)

• I've included appropriate automated tests.
• This is a refactoring of components with existing test coverage.
• Instructions for manual testing are as follows:
  1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

• I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

[jmchilton]

Thanks for the fix - this is clearly a John problem and I will endeavor to find some time to fix up these tests.

[jmchilton]

I was "attempting" to only testing a subset of configurations because I thought all against all on the these combinations would be too much testing for such a niche feature. Does this configuration double the number of tool shed tests we're running?

[nsoranzo]

Yes, from 4 to 8. If you think that some of these combinations are not worth to be run on both Python versions, I can redo the matrix to run them only on Python 3.7.

[nsoranzo]

I initially messed up the second commit (fixed now), so the failing TS tests are actually much less.

Edit: The errors are all due to the randomly occurring playwright._impl._api_types.Error: Navigation interrupted by another one

[jdavcs]

Looks good to me!

[nsoranzo]

@jmchilton Not sure if on purpose, but lib/tool_shed/test/functional/ is currently excluded from mypy testing. After changing:

diff --git a/mypy.ini b/mypy.ini
index fe479abe7d..f9212dbd4e 100644
--- a/mypy.ini
+++ b/mypy.ini
@@ -4,7 +4,11 @@ plugins = pydantic.mypy
 show_error_codes = True
 ignore_missing_imports = True
 check_untyped_defs = True
-exclude = lib/galaxy/tools/bundled|test/functional|.*tool_shed/test/test_data/repos
+exclude = (?x)(
+        ^lib/galaxy/tools/bundled
+        | ^test/functional
+        | .*tool_shed/test/test_data/repos
+    )
 pretty = True
 no_implicit_reexport = True
 no_implicit_optional = True

I get more than 800 mypy errors, even after some fixes added here.

Edit: To clarify, not planning to fix them all here, just a heads-up, will open a separate issue unless that was on purpose.

[nsoranzo]

This is ready from my side, the failing TS tests are either:

• TypeError: string indices must be integers, not 'str', which will be fixed by Restore ToolsApi and create new api route for new panel structure #16872 (confirmed by cherry-picking and testing locally)
• playwright._impl._api_types.Error: Navigation interrupted by another one which are randomly occurring, no idea if they are fixable but shouldn't block this PR.

[mvdbeek]

Awesome, thanks a lot @nsoranzo!

[jmchilton]

.*tool_shed/test/test_data/repos

These are legacy repo contents used for testing that used be locked in tar balls. The repositories I think shouldn't be updated. This isn't code we're maintaining - it is test data that should remain unchanged so we know we're doing the same things.

But maybe I've excluded too much?

[jmchilton]

I'm going to go ahead and merge #16872 then - it looks good to me and it is needed for this.

[nsoranzo]

.*tool_shed/test/test_data/repos

These are legacy repo contents used for testing that used be locked in tar balls. The repositories I think shouldn't be updated. This isn't code we're maintaining - it is test data that should remain unchanged so we know we're doing the same things.

But maybe I've excluded too much?

@jmchilton I think you've misread the diff in my comment above, .*tool_shed/test/test_data/repos is not changed, the important bit is adding ^ in front of test/functional, since otherwise that string is matched anywhere in the file path to be checked therefore excluding also lib/tool_shed/test/functional/ .