diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 461e242..d51a738 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -24,6 +24,7 @@ jobs: run: | python -m pip install --upgrade pip pip install ".[test]" + git submodule update - name: Test with pytest run: | export AWS_DEFAULT_REGION=us-east-1; python -m pytest --cov-report xml --cov=src tests diff --git a/.gitmodules b/.gitmodules new file mode 100644 index 0000000..5f499a5 --- /dev/null +++ b/.gitmodules @@ -0,0 +1,3 @@ +[submodule "tests/test_data/mock_payloads/cumulus-aggregator-test-study"] + path = tests/test_data/mock_payloads/cumulus-aggregator-test-study + url = https://github.com/smart-on-fhir/cumulus-aggregator-test-study diff --git a/src/dashboard/post_distribute/post_distribute.py b/src/dashboard/post_distribute/post_distribute.py index e14735d..3e0ccfe 100644 --- a/src/dashboard/post_distribute/post_distribute.py +++ b/src/dashboard/post_distribute/post_distribute.py @@ -2,6 +2,7 @@ import json import os +import urllib import boto3 import requests @@ -13,8 +14,11 @@ def validate_github_url(config): - if not config["url"].startswith("https://github.com/smart-on-fhir/") or any( - c not in valid_chars for c in config["url"] + parsed_url = urllib.parse.urlparse(config["url"]) + if ( + not parsed_url.netloc == "github.com" + or not parsed_url.path.startswith("/smart-on-fhir") + or any(c not in valid_chars for c in config["url"]) ): raise ValueError(f"{config['url']} is not an official Cumulus study.") res = requests.get(config["url"], timeout=10) diff --git a/src/dashboard/queue_distribute/queue_distribute.py b/src/dashboard/queue_distribute/queue_distribute.py index 383ac23..ce55087 100644 --- a/src/dashboard/queue_distribute/queue_distribute.py +++ b/src/dashboard/queue_distribute/queue_distribute.py @@ -39,12 +39,10 @@ def get_study_from_github(config): try: - subprocess.run(["/usr/bin/git", "clone", config["url"], f"{BASE_DIR}/studies"], check=True) # noqa: S603 - if "tag" in config and config["tag"] is not None: - subprocess.run( # noqa: S603 - ["/usr/bin/git", "checkout", "tag"], - cwd=f"{BASE_DIR}/studies/{config['url'].split('/')[-2]}", - ) + args = ["--depth", "1", config["url"], f"{BASE_DIR}/studies"] + if config["tag"]: + args = ["--branch", config["tag"], *args] + subprocess.run(["/usr/bin/git", "clone", *args], check=True) # noqa: S603 except subprocess.CalledProcessError: # TODO: retry/backoff logic? or do we just have a user queue again? diff --git a/tests/dashboard/test_queue_distribute.py b/tests/dashboard/test_queue_distribute.py index 5135d13..5ad36e0 100644 --- a/tests/dashboard/test_queue_distribute.py +++ b/tests/dashboard/test_queue_distribute.py @@ -25,13 +25,13 @@ def error_callback(process): [ ( "test_study", - "https://github.com/smart-on-fhir/test_study/", + "https://github.com/smart-on-fhir/cumulus-aggregator-test-study/", None, 200, ), ( "test_study", - "https://github.com/smart-on-fhir/test_study/", + "https://github.com/smart-on-fhir/cumulus-aggregator-test-study/", "tag", 200, ), @@ -43,15 +43,23 @@ def test_process_github( mock_notification, tmp_path, name, url, tag, expected_status, monkeypatch, fp ): fp.allow_unregistered(True) # fp is provided by pytest-subprocess + + args = ["--depth", "1", url, f"{tmp_path}/studies"] + if tag: + args = ["--branch", tag, *args] if name == "invalid_study": - fp.register(["/usr/bin/git", "clone", url, f"{tmp_path}/studies"], callback=error_callback) + fp.register(["/usr/bin/git", "clone", *args], callback=error_callback) else: - fp.register(["/usr/bin/git", "clone", url, f"{tmp_path}/studies"]) + fp.register(["/usr/bin/git", "clone", *args]) (tmp_path / "studies").mkdir() + study_dir = tmp_path / f"studies/{name}" shutil.copytree( - pathlib.Path.cwd() / f"./tests/test_data/mock_payloads/{name}/", - tmp_path / f"studies/{name}", + pathlib.Path.cwd() / "./tests/test_data/mock_payloads/cumulus-aggregator-test-study", + study_dir, ) + if tag: + # if we're checking out a tag, make sure we've set up an actual git repo + subprocess.run(["git", "checkout", "tag"], cwd=study_dir) monkeypatch.setattr(queue_distribute, "BASE_DIR", tmp_path) mock_sns = mock.MagicMock() @@ -88,4 +96,5 @@ def test_process_github( if tag == "tag": files = [p for p in (tmp_path / f"studies/{name}").iterdir() if p.is_file()] files = [file.stem for file in files] - assert "tag" in files + print(type(files[0])) + assert "tag" not in files diff --git a/tests/test_data/mock_payloads/cumulus-aggregator-test-study b/tests/test_data/mock_payloads/cumulus-aggregator-test-study new file mode 160000 index 0000000..cfe01c4 --- /dev/null +++ b/tests/test_data/mock_payloads/cumulus-aggregator-test-study @@ -0,0 +1 @@ +Subproject commit cfe01c40a62e706051cde6d10c5c794c66ce026d diff --git a/tests/test_data/mock_payloads/test_study/foo.sql b/tests/test_data/mock_payloads/test_study/foo.sql deleted file mode 100644 index fe23484..0000000 --- a/tests/test_data/mock_payloads/test_study/foo.sql +++ /dev/null @@ -1 +0,0 @@ -CREATE TABLE test_study__foo AS SELECT * FROM bar; \ No newline at end of file diff --git a/tests/test_data/mock_payloads/test_study/foobar.sql b/tests/test_data/mock_payloads/test_study/foobar.sql deleted file mode 100644 index 8c96850..0000000 --- a/tests/test_data/mock_payloads/test_study/foobar.sql +++ /dev/null @@ -1 +0,0 @@ -CREATE TABLE test_study__foobar AS SELECT * FROM bar; \ No newline at end of file diff --git a/tests/test_data/mock_payloads/test_study/manifest.toml b/tests/test_data/mock_payloads/test_study/manifest.toml deleted file mode 100644 index cedc544..0000000 --- a/tests/test_data/mock_payloads/test_study/manifest.toml +++ /dev/null @@ -1,7 +0,0 @@ -study_prefix = "test_study" - -[file_config] -file_names = [ - "foo.sql", - "foobar.sql" -] \ No newline at end of file