From a25c2071064669de6105b2400739a25853a60bbf Mon Sep 17 00:00:00 2001 From: Karan Gathani Date: Mon, 18 Mar 2024 09:38:05 -0700 Subject: [PATCH 01/13] remove unreachable code --- tests/playwright/utils/deploy_utils.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/playwright/utils/deploy_utils.py b/tests/playwright/utils/deploy_utils.py index 71378c9b0..b9a652c87 100644 --- a/tests/playwright/utils/deploy_utils.py +++ b/tests/playwright/utils/deploy_utils.py @@ -148,7 +148,6 @@ def deploy_app( or not (branch_name.startswith("deploy") or branch_name == "main") ): pytest.skip("Not on CI or posit-dev/py-shiny repo or deploy* or main branch") - raise RuntimeError() app_dir = os.path.dirname(app_file_path) write_requirements_txt(app_dir) From 98e09ef4fa1338d39883aedfa871fb355b0f6c1c Mon Sep 17 00:00:00 2001 From: Karan Gathani Date: Mon, 18 Mar 2024 09:55:23 -0700 Subject: [PATCH 02/13] Also run the deploy tests when branch name is main --- .github/workflows/pytest.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pytest.yaml b/.github/workflows/pytest.yaml index b0ac38ed0..e35b4a3d2 100644 --- a/.github/workflows/pytest.yaml +++ b/.github/workflows/pytest.yaml @@ -151,7 +151,7 @@ jobs: playwright-deploys: needs: [playwright-deploys-precheck] - if: github.event_name != 'release' && (github.event_name == 'push' || (github.event_name == 'pull_request' && startsWith(github.head_ref, 'deploy'))) + if: github.event_name != 'release' && (github.event_name == 'push' || (github.event_name == 'pull_request' && (startsWith(github.head_ref, 'deploy') || github.head_ref == 'main'))) # Only allow one `playwright-deploys` job to run at a time. (Independent of branch / PR) # Only one is allowed to run at a time because it is deploying to the same server location. concurrency: playwright-deploys From 0e05b8ad957bf5c18657ea66f591dcefdaea2301 Mon Sep 17 00:00:00 2001 From: Karan Gathani Date: Mon, 18 Mar 2024 10:18:46 -0700 Subject: [PATCH 03/13] just run the deploy tests if it is on CI --- .github/workflows/pytest.yaml | 2 +- tests/playwright/utils/deploy_utils.py | 10 ++-------- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/.github/workflows/pytest.yaml b/.github/workflows/pytest.yaml index e35b4a3d2..b0ac38ed0 100644 --- a/.github/workflows/pytest.yaml +++ b/.github/workflows/pytest.yaml @@ -151,7 +151,7 @@ jobs: playwright-deploys: needs: [playwright-deploys-precheck] - if: github.event_name != 'release' && (github.event_name == 'push' || (github.event_name == 'pull_request' && (startsWith(github.head_ref, 'deploy') || github.head_ref == 'main'))) + if: github.event_name != 'release' && (github.event_name == 'push' || (github.event_name == 'pull_request' && startsWith(github.head_ref, 'deploy'))) # Only allow one `playwright-deploys` job to run at a time. (Independent of branch / PR) # Only one is allowed to run at a time because it is deploying to the same server location. concurrency: playwright-deploys diff --git a/tests/playwright/utils/deploy_utils.py b/tests/playwright/utils/deploy_utils.py index b9a652c87..d2853b6e8 100644 --- a/tests/playwright/utils/deploy_utils.py +++ b/tests/playwright/utils/deploy_utils.py @@ -139,14 +139,8 @@ def deploy_app( pytest.skip("`DEPLOY_APPS` does not equal `true`") run_on_ci = os.environ.get("CI", "False") == "true" - repo = os.environ.get("GITHUB_REPOSITORY", "unknown") - branch_name = os.environ.get("GITHUB_HEAD_REF", "unknown") - - if ( - not run_on_ci - or repo != "posit-dev/py-shiny" - or not (branch_name.startswith("deploy") or branch_name == "main") - ): + + if not run_on_ci: pytest.skip("Not on CI or posit-dev/py-shiny repo or deploy* or main branch") app_dir = os.path.dirname(app_file_path) From ff7939284707ac419468d7b6caa5e6735881c1f6 Mon Sep 17 00:00:00 2001 From: Karan Gathani Date: Mon, 18 Mar 2024 15:05:09 -0700 Subject: [PATCH 04/13] Check if rsconnect json files were updated --- tests/playwright/utils/deploy_utils.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/playwright/utils/deploy_utils.py b/tests/playwright/utils/deploy_utils.py index d2853b6e8..b880c6121 100644 --- a/tests/playwright/utils/deploy_utils.py +++ b/tests/playwright/utils/deploy_utils.py @@ -3,6 +3,7 @@ import json import os import subprocess +import time from typing import Any, Callable, TypeVar import pytest @@ -128,6 +129,18 @@ def write_requirements_txt(app_dir: str) -> None: f.write(f"git+https://github.com/posit-dev/py-shiny.git@{git_hash}\n") +def assert_rsconnect_file_updated(file_path: str, max_minutes: int) -> None: + """ + Asserts that the specified file has been updated within the last `max_minutes` minutes. + """ + current_time = time.time() + mod_time = os.path.getmtime(file_path) + time_diff = (current_time - mod_time) / 60 + assert ( + time_diff < max_minutes + ), f"File '{file_path}' was not updated within the last {max_minutes} minutes which means the deployment failed or was skipped" + + def deploy_app( app_file_path: str, location: str, @@ -152,6 +165,10 @@ def deploy_app( }[location] url = deployment_function(app_name, app_dir) + rsconnect_dir = os.path.join( + app_dir, "rsconnect-python", f"{os.path.basename(app_dir)}.json" + ) + assert_rsconnect_file_updated(rsconnect_dir, 10) return url From 7e8b3ae5b3b035fff4c3c376aca44707e7357d89 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Tue, 19 Mar 2024 14:18:14 -0400 Subject: [PATCH 05/13] Apply suggestions from code review --- tests/playwright/utils/deploy_utils.py | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/tests/playwright/utils/deploy_utils.py b/tests/playwright/utils/deploy_utils.py index b880c6121..e2828025b 100644 --- a/tests/playwright/utils/deploy_utils.py +++ b/tests/playwright/utils/deploy_utils.py @@ -129,16 +129,15 @@ def write_requirements_txt(app_dir: str) -> None: f.write(f"git+https://github.com/posit-dev/py-shiny.git@{git_hash}\n") -def assert_rsconnect_file_updated(file_path: str, max_minutes: int) -> None: +def assert_rsconnect_file_updated(file_path: str, min_mtime: float) -> None: """ - Asserts that the specified file has been updated within the last `max_minutes` minutes. + Asserts that the specified file has been updated since `min_mtime` (seconds since epoch). """ - current_time = time.time() - mod_time = os.path.getmtime(file_path) - time_diff = (current_time - mod_time) / 60 + mtime = os.path.getmtime(file_path) + time_diff = (mtime - min_mtime) assert ( - time_diff < max_minutes - ), f"File '{file_path}' was not updated within the last {max_minutes} minutes which means the deployment failed or was skipped" + mtime > min_mtime + ), f"File '{file_path}' was not updated during app deployment which means the deployment failed" def deploy_app( @@ -152,9 +151,10 @@ def deploy_app( pytest.skip("`DEPLOY_APPS` does not equal `true`") run_on_ci = os.environ.get("CI", "False") == "true" + repo = os.environ.get("GITHUB_REPOSITORY", "unknown") - if not run_on_ci: - pytest.skip("Not on CI or posit-dev/py-shiny repo or deploy* or main branch") + if not (run_on_ci and repo == "posit-dev/py-shiny"): + pytest.skip("Not on CI and within posit-dev/py-shiny repo") app_dir = os.path.dirname(app_file_path) write_requirements_txt(app_dir) @@ -164,11 +164,12 @@ def deploy_app( "shinyapps": quiet_deploy_to_shinyapps, }[location] + pre_deployment_time = time.time() url = deployment_function(app_name, app_dir) rsconnect_dir = os.path.join( app_dir, "rsconnect-python", f"{os.path.basename(app_dir)}.json" ) - assert_rsconnect_file_updated(rsconnect_dir, 10) + assert_rsconnect_file_updated(rsconnect_dir, pre_deployment_time) return url From 1777bcd1d11cf3e3daed5ada88a2b173a4ccd947 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Tue, 19 Mar 2024 15:08:26 -0400 Subject: [PATCH 06/13] Update deploy_utils.py --- tests/playwright/utils/deploy_utils.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/playwright/utils/deploy_utils.py b/tests/playwright/utils/deploy_utils.py index e2828025b..85d8e77d0 100644 --- a/tests/playwright/utils/deploy_utils.py +++ b/tests/playwright/utils/deploy_utils.py @@ -134,7 +134,6 @@ def assert_rsconnect_file_updated(file_path: str, min_mtime: float) -> None: Asserts that the specified file has been updated since `min_mtime` (seconds since epoch). """ mtime = os.path.getmtime(file_path) - time_diff = (mtime - min_mtime) assert ( mtime > min_mtime ), f"File '{file_path}' was not updated during app deployment which means the deployment failed" From 369ca949c769f79ddc36c854419b30a4bd29bbd3 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Tue, 19 Mar 2024 15:12:38 -0400 Subject: [PATCH 07/13] Copy app to temp dir to be able to assert that the rsconnect file was updated --- tests/playwright/utils/deploy_utils.py | 42 ++++++++++++++++++-------- 1 file changed, 30 insertions(+), 12 deletions(-) diff --git a/tests/playwright/utils/deploy_utils.py b/tests/playwright/utils/deploy_utils.py index 85d8e77d0..c0a30db0a 100644 --- a/tests/playwright/utils/deploy_utils.py +++ b/tests/playwright/utils/deploy_utils.py @@ -3,7 +3,10 @@ import json import os import subprocess +import tempfile import time +from distutils.dir_util import copy_tree +from distutils.file_util import copy_file from typing import Any, Callable, TypeVar import pytest @@ -156,20 +159,35 @@ def deploy_app( pytest.skip("Not on CI and within posit-dev/py-shiny repo") app_dir = os.path.dirname(app_file_path) - write_requirements_txt(app_dir) - deployment_function = { - "connect": quiet_deploy_to_connect, - "shinyapps": quiet_deploy_to_shinyapps, - }[location] + # Use temporary directory to avoid modifying the original app directory + # This allows us to run tests in parallel when deploying apps both modify the same rsconnect config file + with tempfile.TemporaryDirectory("deploy_app") as tmpdir: - pre_deployment_time = time.time() - url = deployment_function(app_name, app_dir) - rsconnect_dir = os.path.join( - app_dir, "rsconnect-python", f"{os.path.basename(app_dir)}.json" - ) - assert_rsconnect_file_updated(rsconnect_dir, pre_deployment_time) - return url + copy_tree(app_dir, tmpdir) + + write_requirements_txt(tmpdir) + + deployment_function = { + "connect": quiet_deploy_to_connect, + "shinyapps": quiet_deploy_to_shinyapps, + }[location] + + pre_deployment_time = time.time() + url = deployment_function(app_name, tmpdir) + tmp_rsconnect_dir = os.path.join( + tmpdir, "rsconnect-python", f"{os.path.basename(tmpdir)}.json" + ) + assert_rsconnect_file_updated(tmp_rsconnect_dir, pre_deployment_time) + local_rsconnect_dir = os.path.join( + app_dir, "rsconnect-python", f"{os.path.basename(app_dir)}.json" + ) + # Copy file back if it doesn't exist locally (Helpful for local development and deployment) + if not os.path.exists(local_rsconnect_dir): + + copy_file(tmp_rsconnect_dir, local_rsconnect_dir) + + return url def create_deploys_app_url_fixture( From ba2da88c6df6aeb6daa88b6a8c86ab92d3043997 Mon Sep 17 00:00:00 2001 From: Karan Gathani Date: Thu, 21 Mar 2024 14:18:22 -0700 Subject: [PATCH 08/13] Create new dir within the temporary dir to fix deployment issues --- tests/playwright/utils/deploy_utils.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/tests/playwright/utils/deploy_utils.py b/tests/playwright/utils/deploy_utils.py index c0a30db0a..5ac520b05 100644 --- a/tests/playwright/utils/deploy_utils.py +++ b/tests/playwright/utils/deploy_utils.py @@ -159,14 +159,19 @@ def deploy_app( pytest.skip("Not on CI and within posit-dev/py-shiny repo") app_dir = os.path.dirname(app_file_path) + app_dir_name = os.path.basename(app_dir) # Use temporary directory to avoid modifying the original app directory # This allows us to run tests in parallel when deploying apps both modify the same rsconnect config file with tempfile.TemporaryDirectory("deploy_app") as tmpdir: - copy_tree(app_dir, tmpdir) - - write_requirements_txt(tmpdir) + # Creating a dir with same name instead of tmp to avoid issues + # when deploying app to shinyapps.io using rsconnect package + # since the rsconnect/*.json file needs the app_dir name to be same + tmp_app_dir = os.path.join(tmpdir, app_dir_name) + os.mkdir(tmp_app_dir) + copy_tree(app_dir, tmp_app_dir) + write_requirements_txt(tmp_app_dir) deployment_function = { "connect": quiet_deploy_to_connect, @@ -174,9 +179,9 @@ def deploy_app( }[location] pre_deployment_time = time.time() - url = deployment_function(app_name, tmpdir) + url = deployment_function(app_name, tmp_app_dir) tmp_rsconnect_dir = os.path.join( - tmpdir, "rsconnect-python", f"{os.path.basename(tmpdir)}.json" + tmp_app_dir, "rsconnect-python", f"{os.path.basename(tmp_app_dir)}.json" ) assert_rsconnect_file_updated(tmp_rsconnect_dir, pre_deployment_time) local_rsconnect_dir = os.path.join( From 9ba461b4d6124df917efd708b8d96216149f6961 Mon Sep 17 00:00:00 2001 From: Karan Gathani Date: Fri, 22 Mar 2024 12:39:33 -0700 Subject: [PATCH 09/13] Add distutils typestub --- Makefile | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 76c205aa3..1b7d8c4db 100644 --- a/Makefile +++ b/Makefile @@ -51,6 +51,9 @@ clean-test: ## remove test and coverage artifacts typings/uvicorn: pyright --createstub uvicorn +typings/distutils: + pyright --createstub distutils + typings/matplotlib/__init__.pyi: ## grab type stubs from GitHub mkdir -p typings git clone --depth 1 https://github.com/microsoft/python-type-stubs typings/python-type-stubs @@ -72,7 +75,7 @@ check-black: check-isort: @echo "-------- Sorting imports with isort --------" isort --check-only --diff . -check-types: typings/uvicorn typings/matplotlib/__init__.pyi typings/seaborn +check-types: typings/uvicorn typings/matplotlib/__init__.pyi typings/seaborn typings/distutils @echo "-------- Checking types with pyright --------" pyright check-tests: From 1bbabfe526445347b889f1b4a6ed73b25ba2f8a7 Mon Sep 17 00:00:00 2001 From: Karan Gathani Date: Fri, 22 Mar 2024 13:19:14 -0700 Subject: [PATCH 10/13] use shutils instead of distutils --- Makefile | 5 +---- tests/playwright/utils/deploy_utils.py | 7 +++---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/Makefile b/Makefile index 81a0f73a7..adaedf299 100644 --- a/Makefile +++ b/Makefile @@ -68,9 +68,6 @@ typings/uvicorn: typings/seaborn: @echo "Creating seaborn stubs" pyright --createstub seaborn -typings/distutils: - @echo "Creating seaborn stubs" - pyright --createstub distutils typings/matplotlib/__init__.pyi: @echo "Creating matplotlib stubs" mkdir -p typings @@ -78,7 +75,7 @@ typings/matplotlib/__init__.pyi: mv typings/python-type-stubs/stubs/matplotlib typings/ rm -rf typings/python-type-stubs -pyright-typings: typings/appdirs typings/folium typings/uvicorn typings/seaborn typings/distutils typings/matplotlib/__init__.pyi +pyright-typings: typings/appdirs typings/folium typings/uvicorn typings/seaborn typings/matplotlib/__init__.pyi check: check-format check-lint check-types check-tests ## check code, style, types, and test (basic CI) check-fix: format check-lint check-types check-tests ## check and format code, style, types, and test diff --git a/tests/playwright/utils/deploy_utils.py b/tests/playwright/utils/deploy_utils.py index 5ac520b05..c87dbef7c 100644 --- a/tests/playwright/utils/deploy_utils.py +++ b/tests/playwright/utils/deploy_utils.py @@ -5,8 +5,7 @@ import subprocess import tempfile import time -from distutils.dir_util import copy_tree -from distutils.file_util import copy_file +import shutil from typing import Any, Callable, TypeVar import pytest @@ -170,7 +169,7 @@ def deploy_app( # since the rsconnect/*.json file needs the app_dir name to be same tmp_app_dir = os.path.join(tmpdir, app_dir_name) os.mkdir(tmp_app_dir) - copy_tree(app_dir, tmp_app_dir) + shutil.copytree(app_dir, tmp_app_dir) write_requirements_txt(tmp_app_dir) deployment_function = { @@ -190,7 +189,7 @@ def deploy_app( # Copy file back if it doesn't exist locally (Helpful for local development and deployment) if not os.path.exists(local_rsconnect_dir): - copy_file(tmp_rsconnect_dir, local_rsconnect_dir) + shutil.copy(tmp_rsconnect_dir, local_rsconnect_dir) return url From 9bb9a6ce61cca12d4b7ad6910faa2471a3eddd17 Mon Sep 17 00:00:00 2001 From: Karan Gathani Date: Tue, 26 Mar 2024 06:48:47 -0700 Subject: [PATCH 11/13] sort imports --- tests/playwright/utils/deploy_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/playwright/utils/deploy_utils.py b/tests/playwright/utils/deploy_utils.py index c87dbef7c..1706561f0 100644 --- a/tests/playwright/utils/deploy_utils.py +++ b/tests/playwright/utils/deploy_utils.py @@ -2,10 +2,10 @@ import json import os +import shutil import subprocess import tempfile import time -import shutil from typing import Any, Callable, TypeVar import pytest From d3ed19eb7dbfee8c3803e3085e0c69a14f287d44 Mon Sep 17 00:00:00 2001 From: Karan Gathani Date: Tue, 26 Mar 2024 07:10:08 -0700 Subject: [PATCH 12/13] use dirs_exist_ok flag --- tests/playwright/utils/deploy_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/playwright/utils/deploy_utils.py b/tests/playwright/utils/deploy_utils.py index 1706561f0..68a3cd905 100644 --- a/tests/playwright/utils/deploy_utils.py +++ b/tests/playwright/utils/deploy_utils.py @@ -169,7 +169,7 @@ def deploy_app( # since the rsconnect/*.json file needs the app_dir name to be same tmp_app_dir = os.path.join(tmpdir, app_dir_name) os.mkdir(tmp_app_dir) - shutil.copytree(app_dir, tmp_app_dir) + shutil.copytree(app_dir, tmp_app_dir, dirs_exist_ok=True) write_requirements_txt(tmp_app_dir) deployment_function = { From 383dcfc9c1a2c185ea6548ac62b191d0109b73c4 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Tue, 26 Mar 2024 10:37:03 -0400 Subject: [PATCH 13/13] Apply suggestions from code review --- tests/playwright/utils/deploy_utils.py | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/tests/playwright/utils/deploy_utils.py b/tests/playwright/utils/deploy_utils.py index 68a3cd905..de0bde6f0 100644 --- a/tests/playwright/utils/deploy_utils.py +++ b/tests/playwright/utils/deploy_utils.py @@ -179,17 +179,10 @@ def deploy_app( pre_deployment_time = time.time() url = deployment_function(app_name, tmp_app_dir) - tmp_rsconnect_dir = os.path.join( + tmp_rsconnect_config = os.path.join( tmp_app_dir, "rsconnect-python", f"{os.path.basename(tmp_app_dir)}.json" ) - assert_rsconnect_file_updated(tmp_rsconnect_dir, pre_deployment_time) - local_rsconnect_dir = os.path.join( - app_dir, "rsconnect-python", f"{os.path.basename(app_dir)}.json" - ) - # Copy file back if it doesn't exist locally (Helpful for local development and deployment) - if not os.path.exists(local_rsconnect_dir): - - shutil.copy(tmp_rsconnect_dir, local_rsconnect_dir) + assert_rsconnect_file_updated(tmp_rsconnect_config, pre_deployment_time) return url