From db364dbb3570f4a711697824d37f69a0e1c22924 Mon Sep 17 00:00:00 2001 From: Andrii Ieroshenko Date: Thu, 2 May 2024 17:34:06 -0700 Subject: [PATCH 01/13] make test db a fixture that returns a session, use jp_data_dir fixture --- conftest.py | 69 +++++++++++-------- .../tests/test_execution_manager.py | 25 +++---- jupyter_scheduler/tests/test_scheduler.py | 51 +++++++------- 3 files changed, 73 insertions(+), 72 deletions(-) diff --git a/conftest.py b/conftest.py index 71915074..17e3fd40 100644 --- a/conftest.py +++ b/conftest.py @@ -1,27 +1,56 @@ -import os -from pathlib import Path - import pytest -from jupyter_scheduler.orm import create_session, create_tables +from pathlib import Path +from sqlalchemy import create_engine +from sqlalchemy.orm import sessionmaker + +from jupyter_scheduler.orm import Base from jupyter_scheduler.scheduler import Scheduler from jupyter_scheduler.tests.mocks import MockEnvironmentManager -pytest_plugins = ("jupyter_server.pytest_plugin",) +pytest_plugins = ("jupyter_server.pytest_plugin", "pytest_jupyter.jupyter_server") -HERE = Path(__file__).parent.resolve() -DB_FILE_PATH = f"{HERE}/jupyter_scheduler/tests/testdb.sqlite" -DB_URL = f"sqlite:///{DB_FILE_PATH}" +HERE = Path(__file__).parent.resolve() TEST_ROOT_DIR = f"{HERE}/jupyter_scheduler/tests/test_root_dir" @pytest.fixture -def jp_server_config(jp_server_config): +def jp_scheduler_staging(jp_data_dir): + staging_area = jp_data_dir / "scheduler_staging_area" + staging_area.mkdir() + return staging_area + + +@pytest.fixture +def jp_scheduler_db_url(jp_scheduler_staging): + db_file_path = jp_scheduler_staging / "scheduler.sqlite" + return f"sqlite:///{db_file_path}" + + +@pytest.fixture +def jp_scheduler_db(jp_scheduler_db_url): + engine = create_engine(jp_scheduler_db_url, echo=False) + Base.metadata.create_all(engine) + Session = sessionmaker(bind=engine) + return Session() + + +@pytest.fixture +def jp_scheduler(jp_scheduler_db_url, jp_scheduler_db): + return Scheduler( + db_url=jp_scheduler_db_url, + root_dir=str(TEST_ROOT_DIR), + environments_manager=MockEnvironmentManager(), + ) + + +@pytest.fixture +def jp_server_config(jp_server_config, jp_scheduler_db_url): return { "ServerApp": {"jpserver_extensions": {"jupyter_scheduler": True}}, "SchedulerApp": { - "db_url": DB_URL, + "db_url": jp_scheduler_db_url, "drop_tables": True, "environment_manager_class": "jupyter_scheduler.tests.mocks.MockEnvironmentManager", }, @@ -30,23 +59,3 @@ def jp_server_config(jp_server_config): }, "Scheduler": {"task_runner_class": "jupyter_scheduler.tests.mocks.MockTaskRunner"}, } - - -@pytest.fixture(autouse=True) -def setup_db(): - create_tables(DB_URL, True) - yield - if os.path.exists(DB_FILE_PATH): - os.remove(DB_FILE_PATH) - - -@pytest.fixture -def jp_scheduler_db(): - return create_session(DB_URL) - - -@pytest.fixture -def jp_scheduler(): - return Scheduler( - db_url=DB_URL, root_dir=str(TEST_ROOT_DIR), environments_manager=MockEnvironmentManager() - ) diff --git a/jupyter_scheduler/tests/test_execution_manager.py b/jupyter_scheduler/tests/test_execution_manager.py index a9393eb9..385cf4a6 100644 --- a/jupyter_scheduler/tests/test_execution_manager.py +++ b/jupyter_scheduler/tests/test_execution_manager.py @@ -4,7 +4,6 @@ from sqlalchemy import create_engine from sqlalchemy.orm import sessionmaker -from conftest import DB_URL from jupyter_scheduler.executors import DefaultExecutionManager from jupyter_scheduler.orm import Job @@ -19,25 +18,23 @@ @pytest.fixture def load_job(jp_scheduler_db): - with jp_scheduler_db() as session: - job = Job( - runtime_environment_name="abc", - input_filename=NOTEBOOK_NAME, - job_id=JOB_ID, - ) - session.add(job) - session.commit() + job = Job( + runtime_environment_name="abc", + input_filename=NOTEBOOK_NAME, + job_id=JOB_ID, + ) + jp_scheduler_db.add(job) + jp_scheduler_db.commit() -def test_add_side_effects_files(jp_scheduler_db, load_job): +def test_add_side_effects_files(jp_scheduler_db, load_job, jp_scheduler_db_url): manager = DefaultExecutionManager( job_id=JOB_ID, root_dir=str(NOTEBOOK_DIR), - db_url=DB_URL, + db_url=jp_scheduler_db_url, staging_paths={"input": str(NOTEBOOK_PATH)}, ) manager.add_side_effects_files(str(NOTEBOOK_DIR)) - with jp_scheduler_db() as session: - job = session.query(Job).filter(Job.job_id == JOB_ID).one() - assert SIDE_EFECT_FILE_NAME in job.packaged_files + job = jp_scheduler_db.query(Job).filter(Job.job_id == JOB_ID).one() + assert SIDE_EFECT_FILE_NAME in job.packaged_files diff --git a/jupyter_scheduler/tests/test_scheduler.py b/jupyter_scheduler/tests/test_scheduler.py index fe67504b..d3f68d5b 100644 --- a/jupyter_scheduler/tests/test_scheduler.py +++ b/jupyter_scheduler/tests/test_scheduler.py @@ -127,11 +127,10 @@ def test_create_job_with_input_folder(jp_scheduler): @pytest.fixture def load_job_definitions(jp_scheduler_db): - with jp_scheduler_db() as session: - session.add(JobDefinition(**job_definition_1)) - session.add(JobDefinition(**job_definition_2)) - session.add(JobDefinition(**job_definition_3)) - session.commit() + jp_scheduler_db.add(JobDefinition(**job_definition_1)) + jp_scheduler_db.add(JobDefinition(**job_definition_2)) + jp_scheduler_db.add(JobDefinition(**job_definition_3)) + jp_scheduler_db.commit() @pytest.mark.parametrize( @@ -182,14 +181,13 @@ def test_pause_jobs(jp_scheduler, load_job_definitions, jp_scheduler_db): with patch("jupyter_scheduler.scheduler.Scheduler.task_runner") as mock_task_runner: jp_scheduler.update_job_definition(job_definition_id, UpdateJobDefinition(active=False)) - with jp_scheduler_db() as session: - active = ( - session.query(JobDefinition.active) - .filter(JobDefinition.job_definition_id == job_definition_id) - .one() - .active - ) - assert not active + active = ( + jp_scheduler_db.query(JobDefinition.active) + .filter(JobDefinition.job_definition_id == job_definition_id) + .one() + .active + ) + assert not active def test_resume_jobs(jp_scheduler, load_job_definitions, jp_scheduler_db): @@ -197,14 +195,13 @@ def test_resume_jobs(jp_scheduler, load_job_definitions, jp_scheduler_db): with patch("jupyter_scheduler.scheduler.Scheduler.task_runner") as mock_task_runner: jp_scheduler.update_job_definition(job_definition_id, UpdateJobDefinition(active=True)) - with jp_scheduler_db() as session: - active = ( - session.query(JobDefinition.active) - .filter(JobDefinition.job_definition_id == job_definition_id) - .one() - .active - ) - assert active + active = ( + jp_scheduler_db.query(JobDefinition.active) + .filter(JobDefinition.job_definition_id == job_definition_id) + .one() + .active + ) + assert active def test_update_job_definition(jp_scheduler, load_job_definitions, jp_scheduler_db): @@ -217,15 +214,13 @@ def test_update_job_definition(jp_scheduler, load_job_definitions, jp_scheduler_ ) jp_scheduler.update_job_definition(job_definition_id, update) - with jp_scheduler_db() as session: - definition = session.get(JobDefinition, job_definition_id) - assert schedule == definition.schedule - assert timezone == definition.timezone + definition = jp_scheduler_db.get(JobDefinition, job_definition_id) + assert schedule == definition.schedule + assert timezone == definition.timezone def test_delete_job_definition(jp_scheduler, load_job_definitions, jp_scheduler_db): job_definition_id = job_definition_1["job_definition_id"] jp_scheduler.delete_job_definition(job_definition_id) - with jp_scheduler_db() as session: - definition = session.get(JobDefinition, job_definition_id) - assert not definition + definition = jp_scheduler_db.get(JobDefinition, job_definition_id) + assert not definition From 9539e796f0cca9538f1923e6bab94ab98eea665d Mon Sep 17 00:00:00 2001 From: Andrii Ieroshenko Date: Thu, 2 May 2024 18:15:14 -0700 Subject: [PATCH 02/13] Use job_id returned by the db --- jupyter_scheduler/tests/test_execution_manager.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/jupyter_scheduler/tests/test_execution_manager.py b/jupyter_scheduler/tests/test_execution_manager.py index 385cf4a6..03122c30 100644 --- a/jupyter_scheduler/tests/test_execution_manager.py +++ b/jupyter_scheduler/tests/test_execution_manager.py @@ -7,7 +7,6 @@ from jupyter_scheduler.executors import DefaultExecutionManager from jupyter_scheduler.orm import Job -JOB_ID = "69856f4e-ce94-45fd-8f60-3a587457fce7" NOTEBOOK_NAME = "side_effects.ipynb" SIDE_EFECT_FILE_NAME = "output_side_effect.txt" @@ -17,24 +16,25 @@ @pytest.fixture -def load_job(jp_scheduler_db): +def create_job(jp_scheduler_db): job = Job( runtime_environment_name="abc", input_filename=NOTEBOOK_NAME, - job_id=JOB_ID, ) jp_scheduler_db.add(job) jp_scheduler_db.commit() + return job.job_id -def test_add_side_effects_files(jp_scheduler_db, load_job, jp_scheduler_db_url): +def test_add_side_effects_files(jp_scheduler_db, create_job, jp_scheduler_db_url): + job_id = create_job manager = DefaultExecutionManager( - job_id=JOB_ID, + job_id=job_id, root_dir=str(NOTEBOOK_DIR), db_url=jp_scheduler_db_url, staging_paths={"input": str(NOTEBOOK_PATH)}, ) manager.add_side_effects_files(str(NOTEBOOK_DIR)) - job = jp_scheduler_db.query(Job).filter(Job.job_id == JOB_ID).one() + job = jp_scheduler_db.query(Job).filter(Job.job_id == job_id).one() assert SIDE_EFECT_FILE_NAME in job.packaged_files From 7edf55b067d8ad820fa62f0384ea8a43be7cb591 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 6 May 2024 17:26:54 +0000 Subject: [PATCH 03/13] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- conftest.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/conftest.py b/conftest.py index 17e3fd40..616f85ab 100644 --- a/conftest.py +++ b/conftest.py @@ -1,6 +1,6 @@ -import pytest - from pathlib import Path + +import pytest from sqlalchemy import create_engine from sqlalchemy.orm import sessionmaker From e55be929fbf5433d316713c92aed0668f6c76243 Mon Sep 17 00:00:00 2001 From: Andrii Ieroshenko Date: Mon, 6 May 2024 10:50:29 -0700 Subject: [PATCH 04/13] use fixture for root dir --- conftest.py | 13 ++++++++++--- .../tests/test_execution_manager.py | 18 +++++++++++++----- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/conftest.py b/conftest.py index 616f85ab..4d02018f 100644 --- a/conftest.py +++ b/conftest.py @@ -16,15 +16,22 @@ @pytest.fixture -def jp_scheduler_staging(jp_data_dir): +def jp_scheduler_root_dir(tmp_path): + root_dir = tmp_path / "workspace_root" + root_dir.mkdir() + return root_dir + + +@pytest.fixture +def jp_scheduler_staging_dir(jp_data_dir): staging_area = jp_data_dir / "scheduler_staging_area" staging_area.mkdir() return staging_area @pytest.fixture -def jp_scheduler_db_url(jp_scheduler_staging): - db_file_path = jp_scheduler_staging / "scheduler.sqlite" +def jp_scheduler_db_url(jp_scheduler_staging_dir): + db_file_path = jp_scheduler_staging_dir / "scheduler.sqlite" return f"sqlite:///{db_file_path}" diff --git a/jupyter_scheduler/tests/test_execution_manager.py b/jupyter_scheduler/tests/test_execution_manager.py index 03122c30..a9b49196 100644 --- a/jupyter_scheduler/tests/test_execution_manager.py +++ b/jupyter_scheduler/tests/test_execution_manager.py @@ -16,21 +16,29 @@ @pytest.fixture -def create_job(jp_scheduler_db): +def staging_dir_with_side_effects(jp_scheduler_staging_dir): + return ("side_effects.ipynb", "output_side_effect.txt") + + +@pytest.fixture +def side_effects_job_record(jp_scheduler_db, staging_dir_with_side_effects): + notebook_name = staging_dir_with_side_effects[0] job = Job( runtime_environment_name="abc", - input_filename=NOTEBOOK_NAME, + input_filename=notebook_name, ) jp_scheduler_db.add(job) jp_scheduler_db.commit() return job.job_id -def test_add_side_effects_files(jp_scheduler_db, create_job, jp_scheduler_db_url): - job_id = create_job +def test_add_side_effects_files( + jp_scheduler_db, side_effects_job_record, jp_scheduler_db_url, jp_scheduler_root_dir +): + job_id = side_effects_job_record manager = DefaultExecutionManager( job_id=job_id, - root_dir=str(NOTEBOOK_DIR), + root_dir=jp_scheduler_root_dir, db_url=jp_scheduler_db_url, staging_paths={"input": str(NOTEBOOK_PATH)}, ) From 774b3e92e703b99c40033db61700972aae73df4a Mon Sep 17 00:00:00 2001 From: Andrii Ieroshenko Date: Mon, 6 May 2024 14:35:17 -0700 Subject: [PATCH 05/13] create test dirs by copying test files from static dir into temp dirs instead of uploading test dirs for execution manager test --- conftest.py | 5 +++ .../job-4 => static}/output_side_effect.txt | 0 .../job-4 => static}/side_effects.ipynb | 0 .../tests/test_execution_manager.py | 43 +++++++++++-------- 4 files changed, 30 insertions(+), 18 deletions(-) rename jupyter_scheduler/tests/{test_staging_dir/job-4 => static}/output_side_effect.txt (100%) rename jupyter_scheduler/tests/{test_staging_dir/job-4 => static}/side_effects.ipynb (100%) diff --git a/conftest.py b/conftest.py index 4d02018f..6237d324 100644 --- a/conftest.py +++ b/conftest.py @@ -15,6 +15,11 @@ TEST_ROOT_DIR = f"{HERE}/jupyter_scheduler/tests/test_root_dir" +@pytest.fixture +def static_test_files_dir(): + return HERE / "jupyter_scheduler" / "tests" / "static" + + @pytest.fixture def jp_scheduler_root_dir(tmp_path): root_dir = tmp_path / "workspace_root" diff --git a/jupyter_scheduler/tests/test_staging_dir/job-4/output_side_effect.txt b/jupyter_scheduler/tests/static/output_side_effect.txt similarity index 100% rename from jupyter_scheduler/tests/test_staging_dir/job-4/output_side_effect.txt rename to jupyter_scheduler/tests/static/output_side_effect.txt diff --git a/jupyter_scheduler/tests/test_staging_dir/job-4/side_effects.ipynb b/jupyter_scheduler/tests/static/side_effects.ipynb similarity index 100% rename from jupyter_scheduler/tests/test_staging_dir/job-4/side_effects.ipynb rename to jupyter_scheduler/tests/static/side_effects.ipynb diff --git a/jupyter_scheduler/tests/test_execution_manager.py b/jupyter_scheduler/tests/test_execution_manager.py index a9b49196..3337b2f8 100644 --- a/jupyter_scheduler/tests/test_execution_manager.py +++ b/jupyter_scheduler/tests/test_execution_manager.py @@ -1,48 +1,55 @@ -from pathlib import Path - +import shutil import pytest -from sqlalchemy import create_engine -from sqlalchemy.orm import sessionmaker from jupyter_scheduler.executors import DefaultExecutionManager from jupyter_scheduler.orm import Job -NOTEBOOK_NAME = "side_effects.ipynb" -SIDE_EFECT_FILE_NAME = "output_side_effect.txt" -NOTEBOOK_DIR = Path(__file__).resolve().parent / "test_staging_dir" / "job-4" -NOTEBOOK_PATH = NOTEBOOK_DIR / NOTEBOOK_NAME -SIDE_EFFECT_FILE = NOTEBOOK_DIR / SIDE_EFECT_FILE_NAME +@pytest.fixture +def staging_dir_with_side_effects(static_test_files_dir, jp_scheduler_staging_dir): + notebook_file_path = static_test_files_dir / "side_effects.ipynb" + side_effect_file_path = static_test_files_dir / "output_side_effect.txt" + job_staging_dir = jp_scheduler_staging_dir / "job-4" + job_staging_dir.mkdir() + shutil.copy2(notebook_file_path, job_staging_dir) + shutil.copy2(side_effect_file_path, job_staging_dir) -@pytest.fixture -def staging_dir_with_side_effects(jp_scheduler_staging_dir): - return ("side_effects.ipynb", "output_side_effect.txt") + return (notebook_file_path, side_effect_file_path) @pytest.fixture -def side_effects_job_record(jp_scheduler_db, staging_dir_with_side_effects): - notebook_name = staging_dir_with_side_effects[0] +def side_effects_job_record(staging_dir_with_side_effects, jp_scheduler_db): + notebook_name = staging_dir_with_side_effects[0].name job = Job( runtime_environment_name="abc", input_filename=notebook_name, ) jp_scheduler_db.add(job) jp_scheduler_db.commit() + return job.job_id def test_add_side_effects_files( - jp_scheduler_db, side_effects_job_record, jp_scheduler_db_url, jp_scheduler_root_dir + side_effects_job_record, + staging_dir_with_side_effects, + jp_scheduler_root_dir, + jp_scheduler_db_url, + jp_scheduler_db, ): job_id = side_effects_job_record + staged_notebook_file_path = staging_dir_with_side_effects[0] + staged_notebook_dir = staged_notebook_file_path.parent + side_effect_file_name = staging_dir_with_side_effects[1].name + manager = DefaultExecutionManager( job_id=job_id, root_dir=jp_scheduler_root_dir, db_url=jp_scheduler_db_url, - staging_paths={"input": str(NOTEBOOK_PATH)}, + staging_paths={"input": staged_notebook_file_path}, ) - manager.add_side_effects_files(str(NOTEBOOK_DIR)) + manager.add_side_effects_files(staged_notebook_dir) job = jp_scheduler_db.query(Job).filter(Job.job_id == job_id).one() - assert SIDE_EFECT_FILE_NAME in job.packaged_files + assert side_effect_file_name in job.packaged_files From dd3bc2cd335b23951dcaea87e28de5970506e596 Mon Sep 17 00:00:00 2001 From: Andrii Ieroshenko Date: Mon, 6 May 2024 15:19:21 -0700 Subject: [PATCH 06/13] create test dirs by copying test files from static dir into temp dirs instead of uploading test dirs for scheduler tests --- conftest.py | 12 ++++------ .../job-5/a/b => static}/helloworld.txt | 0 .../job-5 => static}/import-helloworld.ipynb | 0 jupyter_scheduler/tests/test_scheduler.py | 24 +++++++++++++++---- 4 files changed, 24 insertions(+), 12 deletions(-) rename jupyter_scheduler/tests/{test_root_dir/job-5/a/b => static}/helloworld.txt (100%) rename jupyter_scheduler/tests/{test_root_dir/job-5 => static}/import-helloworld.ipynb (100%) diff --git a/conftest.py b/conftest.py index 6237d324..f39a54c7 100644 --- a/conftest.py +++ b/conftest.py @@ -11,13 +11,9 @@ pytest_plugins = ("jupyter_server.pytest_plugin", "pytest_jupyter.jupyter_server") -HERE = Path(__file__).parent.resolve() -TEST_ROOT_DIR = f"{HERE}/jupyter_scheduler/tests/test_root_dir" - - @pytest.fixture def static_test_files_dir(): - return HERE / "jupyter_scheduler" / "tests" / "static" + return Path(__file__).parent.resolve() / "jupyter_scheduler" / "tests" / "static" @pytest.fixture @@ -49,16 +45,16 @@ def jp_scheduler_db(jp_scheduler_db_url): @pytest.fixture -def jp_scheduler(jp_scheduler_db_url, jp_scheduler_db): +def jp_scheduler(jp_scheduler_db_url, jp_scheduler_root_dir, jp_scheduler_db): return Scheduler( db_url=jp_scheduler_db_url, - root_dir=str(TEST_ROOT_DIR), + root_dir=str(jp_scheduler_root_dir), environments_manager=MockEnvironmentManager(), ) @pytest.fixture -def jp_server_config(jp_server_config, jp_scheduler_db_url): +def jp_server_config(jp_scheduler_db_url, jp_server_config): return { "ServerApp": {"jpserver_extensions": {"jupyter_scheduler": True}}, "SchedulerApp": { diff --git a/jupyter_scheduler/tests/test_root_dir/job-5/a/b/helloworld.txt b/jupyter_scheduler/tests/static/helloworld.txt similarity index 100% rename from jupyter_scheduler/tests/test_root_dir/job-5/a/b/helloworld.txt rename to jupyter_scheduler/tests/static/helloworld.txt diff --git a/jupyter_scheduler/tests/test_root_dir/job-5/import-helloworld.ipynb b/jupyter_scheduler/tests/static/import-helloworld.ipynb similarity index 100% rename from jupyter_scheduler/tests/test_root_dir/job-5/import-helloworld.ipynb rename to jupyter_scheduler/tests/static/import-helloworld.ipynb diff --git a/jupyter_scheduler/tests/test_scheduler.py b/jupyter_scheduler/tests/test_scheduler.py index d3f68d5b..2e9c5996 100644 --- a/jupyter_scheduler/tests/test_scheduler.py +++ b/jupyter_scheduler/tests/test_scheduler.py @@ -1,5 +1,7 @@ """Tests for scheduler""" +from pathlib import Path +import shutil from unittest import mock from unittest.mock import patch @@ -16,6 +18,20 @@ from jupyter_scheduler.orm import Job, JobDefinition +@pytest.fixture +def root_dir_with_input_folder(static_test_files_dir, jp_scheduler_root_dir): + notebook_file_path = static_test_files_dir / "import-helloworld.ipynb" + dependency_file_path = static_test_files_dir / "helloworld.txt" + job_root_dir = jp_scheduler_root_dir / "job-5" + dependency_root_dir = job_root_dir / "a" / "b" + + dependency_root_dir.mkdir(parents=True) + shutil.copy2(notebook_file_path, job_root_dir) + shutil.copy2(dependency_file_path, dependency_root_dir) + + return Path(job_root_dir.name) / notebook_file_path.name + + def test_create_job_definition(jp_scheduler): with patch("jupyter_scheduler.scheduler.fsspec") as mock_fsspec: with patch("jupyter_scheduler.scheduler.Scheduler.file_exists") as mock_file_exists: @@ -40,10 +56,10 @@ def test_create_job_definition(jp_scheduler): assert "hello world" == definition.name -def test_create_job_definition_with_input_folder(jp_scheduler): +def test_create_job_definition_with_input_folder(jp_scheduler, root_dir_with_input_folder): job_definition_id = jp_scheduler.create_job_definition( CreateJobDefinition( - input_uri="job-5/import-helloworld.ipynb", + input_uri=str(root_dir_with_input_folder), runtime_environment_name="default", name="import hello world", output_formats=["ipynb"], @@ -61,10 +77,10 @@ def test_create_job_definition_with_input_folder(jp_scheduler): assert "a/b/helloworld.txt" in definition.packaged_files -def test_create_job_with_input_folder(jp_scheduler): +def test_create_job_with_input_folder(jp_scheduler, root_dir_with_input_folder): job_id = jp_scheduler.create_job( CreateJob( - input_uri="job-5/import-helloworld.ipynb", + input_uri=str(root_dir_with_input_folder), runtime_environment_name="default", name="import hello world", output_formats=["ipynb"], From cc61a702008b69a75d5eb8a6cb9b2c6e857ebf89 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 6 May 2024 22:19:40 +0000 Subject: [PATCH 07/13] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- jupyter_scheduler/tests/test_execution_manager.py | 1 + jupyter_scheduler/tests/test_scheduler.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/jupyter_scheduler/tests/test_execution_manager.py b/jupyter_scheduler/tests/test_execution_manager.py index 3337b2f8..9fcb295e 100644 --- a/jupyter_scheduler/tests/test_execution_manager.py +++ b/jupyter_scheduler/tests/test_execution_manager.py @@ -1,4 +1,5 @@ import shutil + import pytest from jupyter_scheduler.executors import DefaultExecutionManager diff --git a/jupyter_scheduler/tests/test_scheduler.py b/jupyter_scheduler/tests/test_scheduler.py index 2e9c5996..54b90d5d 100644 --- a/jupyter_scheduler/tests/test_scheduler.py +++ b/jupyter_scheduler/tests/test_scheduler.py @@ -1,7 +1,7 @@ """Tests for scheduler""" -from pathlib import Path import shutil +from pathlib import Path from unittest import mock from unittest.mock import patch From 655fe69d0e39acd8f6a1f8e14b9cc60055b43491 Mon Sep 17 00:00:00 2001 From: Andrii Ieroshenko Date: Mon, 6 May 2024 22:25:59 -0700 Subject: [PATCH 08/13] create test dirs by copying test files from static dir into temp dirs instead of uploading test dirs for job files manager tests --- conftest.py | 7 ++ .../job-1 => static}/helloworld-1.html | 0 .../job-1 => static}/helloworld-1.ipynb | 0 .../job-1 => static}/helloworld.ipynb | 0 .../job-2 => static}/helloworld.tar.gz | Bin .../tests/test_job_files_manager.py | 107 +++++++++++------- 6 files changed, 72 insertions(+), 42 deletions(-) rename jupyter_scheduler/tests/{test_staging_dir/job-1 => static}/helloworld-1.html (100%) rename jupyter_scheduler/tests/{test_staging_dir/job-1 => static}/helloworld-1.ipynb (100%) rename jupyter_scheduler/tests/{test_staging_dir/job-1 => static}/helloworld.ipynb (100%) rename jupyter_scheduler/tests/{test_staging_dir/job-2 => static}/helloworld.tar.gz (100%) diff --git a/conftest.py b/conftest.py index f39a54c7..253b796f 100644 --- a/conftest.py +++ b/conftest.py @@ -23,6 +23,13 @@ def jp_scheduler_root_dir(tmp_path): return root_dir +@pytest.fixture +def jp_scheduler_output_dir(jp_scheduler_root_dir): + output_dir = jp_scheduler_root_dir / "jobs" + output_dir.mkdir() + return output_dir + + @pytest.fixture def jp_scheduler_staging_dir(jp_data_dir): staging_area = jp_data_dir / "scheduler_staging_area" diff --git a/jupyter_scheduler/tests/test_staging_dir/job-1/helloworld-1.html b/jupyter_scheduler/tests/static/helloworld-1.html similarity index 100% rename from jupyter_scheduler/tests/test_staging_dir/job-1/helloworld-1.html rename to jupyter_scheduler/tests/static/helloworld-1.html diff --git a/jupyter_scheduler/tests/test_staging_dir/job-1/helloworld-1.ipynb b/jupyter_scheduler/tests/static/helloworld-1.ipynb similarity index 100% rename from jupyter_scheduler/tests/test_staging_dir/job-1/helloworld-1.ipynb rename to jupyter_scheduler/tests/static/helloworld-1.ipynb diff --git a/jupyter_scheduler/tests/test_staging_dir/job-1/helloworld.ipynb b/jupyter_scheduler/tests/static/helloworld.ipynb similarity index 100% rename from jupyter_scheduler/tests/test_staging_dir/job-1/helloworld.ipynb rename to jupyter_scheduler/tests/static/helloworld.ipynb diff --git a/jupyter_scheduler/tests/test_staging_dir/job-2/helloworld.tar.gz b/jupyter_scheduler/tests/static/helloworld.tar.gz similarity index 100% rename from jupyter_scheduler/tests/test_staging_dir/job-2/helloworld.tar.gz rename to jupyter_scheduler/tests/static/helloworld.tar.gz diff --git a/jupyter_scheduler/tests/test_job_files_manager.py b/jupyter_scheduler/tests/test_job_files_manager.py index e6fcb5d6..66a9727b 100644 --- a/jupyter_scheduler/tests/test_job_files_manager.py +++ b/jupyter_scheduler/tests/test_job_files_manager.py @@ -60,65 +60,88 @@ async def test_copy_from_staging(): ) -HERE = Path(__file__).parent.resolve() -OUTPUTS_DIR = os.path.join(HERE, "test_files_output") +@pytest.fixture +def staging_dir_with_notebook_job(static_test_files_dir, jp_scheduler_staging_dir): + staging_dir = jp_scheduler_staging_dir / "job-1" + job_filenames = ["helloworld-1.ipynb", "helloworld-1.html", "helloworld.ipynb"] + + staged_job_files = [] + staging_dir.mkdir() + for job_filename in job_filenames: + staged_job_file = shutil.copy2(static_test_files_dir / job_filename, staging_dir) + staged_job_files.append(staged_job_file) + + return staged_job_files @pytest.fixture -def clear_outputs_dir(): - yield - shutil.rmtree(OUTPUTS_DIR) - # rmtree() is not synchronous; wait until it has finished running - while os.path.isdir(OUTPUTS_DIR): - time.sleep(0.01) - - -@pytest.mark.parametrize( - "output_formats, output_filenames, staging_paths, output_dir, redownload", - [ - ( - ["ipynb", "html"], - { +def staging_dir_with_tar_job(static_test_files_dir, jp_scheduler_staging_dir): + staging_dir = jp_scheduler_staging_dir / "job-2" + job_tar_file = static_test_files_dir / "helloworld.tar.gz" + + staging_dir.mkdir() + staged_tar_file = shutil.copy2(job_tar_file, staging_dir) + + return staged_tar_file + + +@pytest.fixture +def downloader_parameters( + staging_dir_with_notebook_job, + staging_dir_with_tar_job, + request, + jp_scheduler_output_dir, +): + job_1_ipynb_file_path, job_1_html_file_path, job_1_input_file_path = ( + staging_dir_with_notebook_job + ) + job_2_tar_file_path = staging_dir_with_tar_job + index = request.param + parameters = [ + { + "output_formats": ["ipynb", "html"], + "output_filenames": { "ipynb": "job-1/helloworld-out.ipynb", "html": "job-1/helloworld-out.html", "input": "job-1/helloworld-input.ipynb", }, - { - "ipynb": os.path.join(HERE, "test_staging_dir", "job-1", "helloworld-1.ipynb"), - "html": os.path.join(HERE, "test_staging_dir", "job-1", "helloworld-1.html"), - "input": os.path.join(HERE, "test_staging_dir", "job-1", "helloworld.ipynb"), + "staging_paths": { + "ipynb": job_1_ipynb_file_path, + "html": job_1_html_file_path, + "input": job_1_input_file_path, }, - OUTPUTS_DIR, - False, - ), - ( - ["ipynb", "html"], - { + "output_dir": jp_scheduler_output_dir, + "redownload": False, + }, + { + "output_formats": ["ipynb", "html"], + "output_filenames": { "ipynb": "job-2/helloworld-1.ipynb", "html": "job-2/helloworld-1.html", "input": "job-2/helloworld.ipynb", }, - { - "tar.gz": os.path.join(HERE, "test_staging_dir", "job-2", "helloworld.tar.gz"), + "staging_paths": { + "tar.gz": job_2_tar_file_path, "ipynb": "job-2/helloworld-1.ipynb", "html": "job-2/helloworld-1.html", "input": "job-2/helloworld.ipynb", }, - OUTPUTS_DIR, - False, - ), - ], -) -def test_downloader_download( - clear_outputs_dir, output_formats, output_filenames, staging_paths, output_dir, redownload -): - downloader = Downloader( - output_formats=output_formats, - output_filenames=output_filenames, - staging_paths=staging_paths, - output_dir=output_dir, - redownload=redownload, + "output_dir": jp_scheduler_output_dir, + "redownload": False, + }, + ] + return parameters[index] + + +@pytest.mark.parametrize("downloader_parameters", [0, 1], indirect=True) +def test_downloader_download(downloader_parameters): + output_formats, output_filenames, staging_paths, output_dir = ( + downloader_parameters["output_formats"], + downloader_parameters["output_filenames"], + downloader_parameters["staging_paths"], + downloader_parameters["output_dir"], ) + downloader = Downloader(**downloader_parameters) downloader.download() assert os.path.exists(output_dir) From bc1d170325c5eb23f341abd9077749540c65f152 Mon Sep 17 00:00:00 2001 From: Andrii Ieroshenko Date: Tue, 7 May 2024 09:31:42 -0700 Subject: [PATCH 09/13] make static_test_files_dir fixture session-scoped --- conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/conftest.py b/conftest.py index 253b796f..8683b42a 100644 --- a/conftest.py +++ b/conftest.py @@ -11,7 +11,7 @@ pytest_plugins = ("jupyter_server.pytest_plugin", "pytest_jupyter.jupyter_server") -@pytest.fixture +@pytest.fixture(scope="session") def static_test_files_dir(): return Path(__file__).parent.resolve() / "jupyter_scheduler" / "tests" / "static" From c30e70481d3cfff49af70f7094e325ba53888628 Mon Sep 17 00:00:00 2001 From: Andrii Ieroshenko Date: Tue, 14 May 2024 10:26:48 -0700 Subject: [PATCH 10/13] add return type annotations to fixtures where return type is not clear --- conftest.py | 14 +++++++------- jupyter_scheduler/tests/test_execution_manager.py | 8 ++++++-- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/conftest.py b/conftest.py index 8683b42a..27edd4f0 100644 --- a/conftest.py +++ b/conftest.py @@ -2,7 +2,7 @@ import pytest from sqlalchemy import create_engine -from sqlalchemy.orm import sessionmaker +from sqlalchemy.orm import sessionmaker, Session from jupyter_scheduler.orm import Base from jupyter_scheduler.scheduler import Scheduler @@ -12,39 +12,39 @@ @pytest.fixture(scope="session") -def static_test_files_dir(): +def static_test_files_dir() -> Path: return Path(__file__).parent.resolve() / "jupyter_scheduler" / "tests" / "static" @pytest.fixture -def jp_scheduler_root_dir(tmp_path): +def jp_scheduler_root_dir(tmp_path) -> Path: root_dir = tmp_path / "workspace_root" root_dir.mkdir() return root_dir @pytest.fixture -def jp_scheduler_output_dir(jp_scheduler_root_dir): +def jp_scheduler_output_dir(jp_scheduler_root_dir) -> Path: output_dir = jp_scheduler_root_dir / "jobs" output_dir.mkdir() return output_dir @pytest.fixture -def jp_scheduler_staging_dir(jp_data_dir): +def jp_scheduler_staging_dir(jp_data_dir) -> Path: staging_area = jp_data_dir / "scheduler_staging_area" staging_area.mkdir() return staging_area @pytest.fixture -def jp_scheduler_db_url(jp_scheduler_staging_dir): +def jp_scheduler_db_url(jp_scheduler_staging_dir) -> str: db_file_path = jp_scheduler_staging_dir / "scheduler.sqlite" return f"sqlite:///{db_file_path}" @pytest.fixture -def jp_scheduler_db(jp_scheduler_db_url): +def jp_scheduler_db(jp_scheduler_db_url) -> Session: engine = create_engine(jp_scheduler_db_url, echo=False) Base.metadata.create_all(engine) Session = sessionmaker(bind=engine) diff --git a/jupyter_scheduler/tests/test_execution_manager.py b/jupyter_scheduler/tests/test_execution_manager.py index 9fcb295e..66432412 100644 --- a/jupyter_scheduler/tests/test_execution_manager.py +++ b/jupyter_scheduler/tests/test_execution_manager.py @@ -1,4 +1,6 @@ +from pathlib import Path import shutil +from typing import Tuple import pytest @@ -7,7 +9,9 @@ @pytest.fixture -def staging_dir_with_side_effects(static_test_files_dir, jp_scheduler_staging_dir): +def staging_dir_with_side_effects( + static_test_files_dir, jp_scheduler_staging_dir +) -> Tuple[Path, Path]: notebook_file_path = static_test_files_dir / "side_effects.ipynb" side_effect_file_path = static_test_files_dir / "output_side_effect.txt" job_staging_dir = jp_scheduler_staging_dir / "job-4" @@ -20,7 +24,7 @@ def staging_dir_with_side_effects(static_test_files_dir, jp_scheduler_staging_di @pytest.fixture -def side_effects_job_record(staging_dir_with_side_effects, jp_scheduler_db): +def side_effects_job_record(staging_dir_with_side_effects, jp_scheduler_db) -> str: notebook_name = staging_dir_with_side_effects[0].name job = Job( runtime_environment_name="abc", From 160dc5fbd579e8265e0e32f4825cb81eca9cd9bb Mon Sep 17 00:00:00 2001 From: Andrii Ieroshenko Date: Tue, 14 May 2024 10:32:19 -0700 Subject: [PATCH 11/13] explicitly close db session to ensure all db connections are released --- conftest.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/conftest.py b/conftest.py index 27edd4f0..8885cf6a 100644 --- a/conftest.py +++ b/conftest.py @@ -44,11 +44,13 @@ def jp_scheduler_db_url(jp_scheduler_staging_dir) -> str: @pytest.fixture -def jp_scheduler_db(jp_scheduler_db_url) -> Session: +def jp_scheduler_db(jp_scheduler_db_url): engine = create_engine(jp_scheduler_db_url, echo=False) Base.metadata.create_all(engine) Session = sessionmaker(bind=engine) - return Session() + session = Session() + yield session + session.close() @pytest.fixture From fba68837fe395a20bbe6070d0feaa6599569e26b Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 14 May 2024 17:35:24 +0000 Subject: [PATCH 12/13] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- conftest.py | 2 +- jupyter_scheduler/tests/test_execution_manager.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/conftest.py b/conftest.py index 8885cf6a..e675cbc2 100644 --- a/conftest.py +++ b/conftest.py @@ -2,7 +2,7 @@ import pytest from sqlalchemy import create_engine -from sqlalchemy.orm import sessionmaker, Session +from sqlalchemy.orm import Session, sessionmaker from jupyter_scheduler.orm import Base from jupyter_scheduler.scheduler import Scheduler diff --git a/jupyter_scheduler/tests/test_execution_manager.py b/jupyter_scheduler/tests/test_execution_manager.py index 66432412..66546be3 100644 --- a/jupyter_scheduler/tests/test_execution_manager.py +++ b/jupyter_scheduler/tests/test_execution_manager.py @@ -1,5 +1,5 @@ -from pathlib import Path import shutil +from pathlib import Path from typing import Tuple import pytest From c466bd509963dc3a186f2397b7c3c16a7ee05172 Mon Sep 17 00:00:00 2001 From: Andrii Ieroshenko Date: Tue, 14 May 2024 11:01:14 -0700 Subject: [PATCH 13/13] Assert tmp_path and similar as Path type --- conftest.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/conftest.py b/conftest.py index e675cbc2..87cebe69 100644 --- a/conftest.py +++ b/conftest.py @@ -17,28 +17,28 @@ def static_test_files_dir() -> Path: @pytest.fixture -def jp_scheduler_root_dir(tmp_path) -> Path: +def jp_scheduler_root_dir(tmp_path: Path) -> Path: root_dir = tmp_path / "workspace_root" root_dir.mkdir() return root_dir @pytest.fixture -def jp_scheduler_output_dir(jp_scheduler_root_dir) -> Path: +def jp_scheduler_output_dir(jp_scheduler_root_dir: Path) -> Path: output_dir = jp_scheduler_root_dir / "jobs" output_dir.mkdir() return output_dir @pytest.fixture -def jp_scheduler_staging_dir(jp_data_dir) -> Path: +def jp_scheduler_staging_dir(jp_data_dir: Path) -> Path: staging_area = jp_data_dir / "scheduler_staging_area" staging_area.mkdir() return staging_area @pytest.fixture -def jp_scheduler_db_url(jp_scheduler_staging_dir) -> str: +def jp_scheduler_db_url(jp_scheduler_staging_dir: Path) -> str: db_file_path = jp_scheduler_staging_dir / "scheduler.sqlite" return f"sqlite:///{db_file_path}"