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

Tool Shed 2.0 fixes #16825

Merged
merged 19 commits into from
Oct 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 20 additions & 23 deletions .github/workflows/toolshed.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,24 +19,11 @@ jobs:
name: Test
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
include:
- test-install-client: 'galaxy_api'
python-version: '3.7'
shed-api: 'v1'
shed-browser: 'twill'
- test-install-client: 'standalone'
python-version: '3.8'
shed-api: 'v1'
shed-browser: 'twill'
- test-install-client: 'galaxy_api'
python-version: '3.9'
shed-api: 'v2'
shed-browser: 'playwright'
- test-install-client: 'standalone'
python-version: '3.10'
shed-api: 'v2'
shed-browser: 'playwright'
Copy link
Member

Choose a reason for hiding this comment

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

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?

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

python-version: ['3.7', '3.11']
shed-api: ['v1', 'v2']
test-install-client: ['galaxy_api', 'standalone']
services:
postgres:
image: postgres:13
Expand All @@ -50,6 +37,11 @@ jobs:
- uses: actions/checkout@v3
with:
path: 'galaxy root'
- uses: actions/setup-node@v3
with:
node-version: '18.12.1'
cache: 'yarn'
cache-dependency-path: 'galaxy root/client/yarn.lock'
- uses: actions/setup-python@v4
with:
python-version: ${{ matrix.python-version }}
Expand All @@ -68,25 +60,30 @@ jobs:
with:
path: 'galaxy root/.venv'
key: gxy-venv-${{ runner.os }}-${{ steps.full-python-version.outputs.version }}-${{ hashFiles('galaxy root/requirements.txt') }}-toolshed
key: gxy-venv-${{ runner.os }}-${{ steps.full-python-version.outputs.version }}-${{ hashFiles('galaxy_root/requirements.txt') }}-toolshed
- name: Install dependencies
run: ./scripts/common_startup.sh --skip-client-build
working-directory: 'galaxy root'
- name: Build Frontend
run: '. .venv/bin/activate && cd lib/tool_shed/webapp/frontend && yarn && make client'
run: |
. .venv/bin/activate
cd lib/tool_shed/webapp/frontend
yarn
make client
working-directory: 'galaxy root'
- name: Install playwright
run: '. .venv/bin/activate && playwright install'
run: |
. .venv/bin/activate
playwright install
working-directory: 'galaxy root'
- name: Run tests
run: './run_tests.sh -toolshed'
run: ./run_tests.sh -toolshed
env:
TOOL_SHED_TEST_INSTALL_CLIENT: ${{ matrix.test-install-client }}
TOOL_SHED_API_VERSION: ${{ matrix.shed-api }}
TOOL_SHED_TEST_BROWSER: ${{ matrix.shed-browser }}
TOOL_SHED_TEST_BROWSER: ${{ matrix.shed-api == 'v1' && 'twill' || 'playwright' }}
working-directory: 'galaxy root'
- uses: actions/upload-artifact@v3
if: failure()
with:
name: Toolshed test results (${{ matrix.python-version }}, ${{ matrix.test-install-client }})
name: Toolshed test results (${{ matrix.python-version }}, ${{ matrix.shed-api }}, ${{ matrix.test-install-client }})
path: 'galaxy root/run_toolshed_tests.html'
1 change: 0 additions & 1 deletion lib/galaxy_test/driver/driver_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,6 @@ def setup_galaxy_config(
template_cache_path=template_cache_path,
tool_config_file=tool_config_file,
tool_data_table_config_path=tool_data_table_config_path,
tool_parse_help=False,
tool_path=tool_path,
update_integrated_tool_panel=update_integrated_tool_panel,
use_tasked_jobs=True,
Expand Down
4 changes: 4 additions & 0 deletions lib/tool_shed/managers/categories.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ class CategoryManager:
def __init__(self, app: ToolShedApp):
self.app = app

def get(self, encoded_category_id: str) -> Category:
return suc.get_category(self.app, encoded_category_id)

def create(self, trans: ProvidesUserContext, category_request: CreateCategoryRequest) -> Category:
name = category_request.name
description = category_request.description or name
Expand Down Expand Up @@ -71,6 +74,7 @@ def to_model(self, category: Category) -> CategoryResponse:
name=as_dict["name"],
description=as_dict["description"],
repositories=as_dict["repositories"],
deleted=as_dict["deleted"],
)


Expand Down
15 changes: 10 additions & 5 deletions lib/tool_shed/managers/repositories.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
false,
select,
)
from sqlalchemy.orm import scoped_session

from galaxy import web
from galaxy.exceptions import (
Expand Down Expand Up @@ -254,7 +255,9 @@ def index_tool_ids(app: ToolShedApp, tool_ids: List[str]) -> Dict[str, Any]:


def index_repositories(app: ToolShedApp, name: Optional[str], owner: Optional[str], deleted: bool):
return list(_get_repository_by_name_and_owner_and_deleted(app.model.context, name, owner, deleted, app.model.User))
return list(
_get_repositories_by_name_and_owner_and_deleted(app.model.context, name, owner, deleted, app.model.User)
)


def can_manage_repo(trans: ProvidesUserContext, repository: Repository) -> bool:
Expand Down Expand Up @@ -579,7 +582,7 @@ def upload_tar_and_set_metadata(
return message


def _get_repository_by_name_and_owner(session, name, owner, user_model):
def _get_repository_by_name_and_owner(session: scoped_session, name: str, owner: str, user_model):
stmt = (
select(Repository)
.where(Repository.deprecated == false())
Expand All @@ -592,12 +595,14 @@ def _get_repository_by_name_and_owner(session, name, owner, user_model):
return session.scalars(stmt).first()


def _get_repository_by_name_and_owner_and_deleted(session, name, owner, deleted, user_model):
def _get_repositories_by_name_and_owner_and_deleted(
session: scoped_session, name: Optional[str], owner: Optional[str], deleted: bool, user_model
):
stmt = select(Repository).where(Repository.deprecated == false()).where(Repository.deleted == deleted)
if owner is not None:
stmt = stmt.where(user_model.username == owner)
stmt = stmt.where(Repository.user_id == user_model.id)
if name is not None:
stmt = stmt.where(Repository.name == name)
stmt = stmt.order_by(Repository.name).limit(1)
return session.scalars(stmt).first()
stmt = stmt.order_by(Repository.name)
return session.scalars(stmt)
2 changes: 1 addition & 1 deletion lib/tool_shed/metadata/repository_metadata_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -1097,7 +1097,7 @@ def get_current_repositories(session, order=False):


def get_filtered_repositories(session, repo_ids, order):
stmt = select(Repository).where(Repository.in_(repo_ids))
stmt = select(Repository).where(Repository.id.in_(repo_ids))
if order:
stmt = stmt.order_by(Repository.name, Repository.user_id)
return session.scalars(stmt)
6 changes: 3 additions & 3 deletions lib/tool_shed/test/base/driver.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,11 @@ def _setup_local(self):
os.environ["GALAXY_TEST_TOOL_DATA_PATH"] = tool_data_path
galaxy_db_path = driver_util.database_files_path(tool_shed_test_tmp_dir)
shed_file_path = os.path.join(shed_db_path, "files")
hgweb_config_file_path = tempfile.mkdtemp(dir=tool_shed_test_tmp_dir)
whoosh_index_dir = os.path.join(shed_db_path, "toolshed_whoosh_indexes")
hgweb_config_dir = tempfile.mkdtemp(dir=tool_shed_test_tmp_dir)
new_repos_path = tempfile.mkdtemp(dir=tool_shed_test_tmp_dir)
galaxy_shed_tool_path = tempfile.mkdtemp(dir=tool_shed_test_tmp_dir)
galaxy_migrated_tool_path = tempfile.mkdtemp(dir=tool_shed_test_tmp_dir)
hgweb_config_dir = hgweb_config_file_path
os.environ["TEST_HG_WEB_CONFIG_DIR"] = hgweb_config_dir
print("Directory location for hgweb.config:", hgweb_config_dir)
toolshed_database_conf = driver_util.database_conf(shed_db_path, prefix="TOOL_SHED")
Expand All @@ -103,8 +103,8 @@ def _setup_local(self):
shed_tool_data_table_config=shed_tool_data_table_conf_file,
smtp_server="smtp.dummy.string.tld",
email_from="functional@localhost",
tool_parse_help=False,
use_heartbeat=False,
whoosh_index_dir=whoosh_index_dir,
)
kwargs.update(toolshed_database_conf)
# Generate the tool_data_table_conf.xml file.
Expand Down
7 changes: 5 additions & 2 deletions lib/tool_shed/test/base/populators.py
Original file line number Diff line number Diff line change
Expand Up @@ -266,9 +266,12 @@ def get_categories(self) -> List[Category]:
response.raise_for_status()
return [Category(**c) for c in response.json()]

def get_category_with_name(self, name: str) -> Optional[Category]:
response = self._api_interactor.get("categories")
def get_category_with_id(self, category_id: str) -> Category:
response = self._api_interactor.get(f"categories/{category_id}")
response.raise_for_status()
return Category(**response.json())

def get_category_with_name(self, name: str) -> Optional[Category]:
categories = [c for c in self.get_categories() if c.name == name]
return categories[0] if categories else None

Expand Down
18 changes: 8 additions & 10 deletions lib/tool_shed/test/base/twilltestcase.py
Original file line number Diff line number Diff line change
Expand Up @@ -764,7 +764,7 @@ def login(
)
# v2 doesn't log you in on account creation... so force a login here
if previously_created or self.is_v2:
# The acount has previously been created, so just login.
# The account has previously been created, so just login.
# HACK: don't use panels because late_javascripts() messes up the twill browser and it
# can't find form fields (and hence user can't be logged in).
params = {"use_panels": False}
Expand Down Expand Up @@ -1040,7 +1040,7 @@ def create_category(self, **kwd) -> Category:

def create_repository_dependency(
self,
repository: Optional[Repository] = None,
repository: Repository,
repository_tuples=None,
filepath=None,
prior_installation_required=False,
Expand All @@ -1050,7 +1050,6 @@ def create_repository_dependency(
strings_displayed=None,
strings_not_displayed=None,
):
assert repository
repository_tuples = repository_tuples or []
repository_names = []
if complex:
Expand Down Expand Up @@ -1402,21 +1401,20 @@ def get_repositories_category_api(
self.check_for_strings(strings_displayed, strings_not_displayed)

def get_or_create_repository(
self, category: Category, owner=None, strings_displayed=None, strings_not_displayed=None, **kwd
self, category: Category, owner: str, name: str, strings_displayed=None, strings_not_displayed=None, **kwd
) -> Optional[Repository]:
# If not checking for a specific string, it should be safe to assume that
# we expect repository creation to be successful.
if strings_displayed is None:
strings_displayed = ["Repository", kwd["name"], "has been created"]
strings_displayed = ["Repository", name, "has been created"]
if strings_not_displayed is None:
strings_not_displayed = []
name = kwd["name"]
repository = self.populator.get_repository_for(owner, name)
category_id = category.id
assert category_id
if repository is None:
category_id = category.id
assert category_id
self.visit_url("/repository/create_repository")
self.submit_form(button="create_repository_button", category_id=category_id, **kwd)
self.submit_form(button="create_repository_button", name=name, category_id=category_id, **kwd)
self.check_for_strings(strings_displayed, strings_not_displayed)
repository = self.populator.get_repository_for(owner, name)
return repository
Expand Down Expand Up @@ -2093,7 +2091,7 @@ def get_installed_repository(session, name, owner, changeset):
if owner is not None:
stmt = stmt.where(ToolShedRepository.owner == owner)
if changeset is not None:
stmt = stmt.wehre(ToolShedRepository.changeset_revision == changeset)
stmt = stmt.where(ToolShedRepository.changeset_revision == changeset)
stmt = stmt.where(ToolShedRepository.deleted == false())
stmt = stmt.where(ToolShedRepository.uninstalled == false())
return session.scalars(stmt).one_or_none()
10 changes: 6 additions & 4 deletions lib/tool_shed/test/functional/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,12 @@
Browser,
BrowserContext,
)
from typing_extensions import Literal

from ..base.browser import ShedBrowser
from ..base.playwrightbrowser import PlaywrightShedBrowser
from ..base.twillbrowser import TwillShedBrowser

DEFAULT_BROWSER: Literal["twill", "playwright"] = "playwright"
DEFAULT_BROWSER = "playwright"


def twill_browser() -> Generator[ShedBrowser, None, None]:
Expand All @@ -28,10 +27,13 @@ def playwright_browser(class_context: BrowserContext) -> Generator[ShedBrowser,
yield PlaywrightShedBrowser(page)


if os.environ.get("TOOL_SHED_TEST_BROWSER", DEFAULT_BROWSER) == "twill":
test_browser = os.environ.get("TOOL_SHED_TEST_BROWSER", DEFAULT_BROWSER)
if test_browser == "twill":
shed_browser = pytest.fixture(scope="class")(twill_browser)
else:
elif test_browser == "playwright":
shed_browser = pytest.fixture(scope="class")(playwright_browser)
else:
raise ValueError(f"Unrecognized value for TOOL_SHED_TEST_BROWSER: {test_browser}")


@pytest.fixture(scope="class")
Expand Down
Loading
Loading