From 6fdd8d4e914938db1a2323b85bf463da4a7e2330 Mon Sep 17 00:00:00 2001 From: Christopher DeCarolis Date: Thu, 19 Sep 2024 08:28:38 -0700 Subject: [PATCH] [dagster-airlift] fix test fixture inheritance (#24582) Fix some nonsense about pytest fixture inheritance that was causing a misleading error to appear in the output. Also use better convention for makefile format. --- .../examples/dbt-example/Makefile | 22 +++++++++---------- .../integration_tests/conftest.py | 2 +- .../examples/perf-harness/Makefile | 14 ++++++------ .../perf_harness_tests/conftest.py | 22 ++++++++++++++----- 4 files changed, 36 insertions(+), 24 deletions(-) diff --git a/examples/experimental/dagster-airlift/examples/dbt-example/Makefile b/examples/experimental/dagster-airlift/examples/dbt-example/Makefile index 1efedef5670ba..4cf5095417cbc 100644 --- a/examples/experimental/dagster-airlift/examples/dbt-example/Makefile +++ b/examples/experimental/dagster-airlift/examples/dbt-example/Makefile @@ -4,7 +4,7 @@ define GET_MAKEFILE_DIR $(shell dirname $(realpath $(lastword $(MAKEFILE_LIST))) | sed 's:/*$$::') endef -MAKEFILE_DIR := $(GET_MAKEFILE_DIR) +export MAKEFILE_DIR := $(GET_MAKEFILE_DIR) export DAGSTER_HOME := $(MAKEFILE_DIR)/.dagster_home export AIRFLOW_HOME := $(MAKEFILE_DIR)/.airflow_home export DAGSTER_AIRLIFT_MIGRATION_STATE_DIR := $(MAKEFILE_DIR)/dbt_example/airflow_dags/migration_state @@ -23,13 +23,13 @@ dev_install: uv pip install -e ../../../dagster-airlift uv pip install -e . -setup_local_env: - make wipe && \ - mkdir -p $$AIRFLOW_HOME && \ - mkdir -p $$DAGSTER_HOME && \ - chmod +x ../../scripts/airflow_setup.sh && \ - ../../scripts/airflow_setup.sh $(MAKEFILE_DIR)/dbt_example/airflow_dags && \ - make dbt_setup +setup_local_env: + $(MAKE) wipe + mkdir -p $(AIRFLOW_HOME) + mkdir -p $(DAGSTER_HOME) + chmod +x ../../scripts/airflow_setup.sh + ../../scripts/airflow_setup.sh $(MAKEFILE_DIR)/dbt_example/airflow_dags + $(MAKE) dbt_setup run_airflow: airflow standalone @@ -41,8 +41,8 @@ run_peer: dagster dev -m dbt_example.dagster_defs.peer -p 3333 run_observe: - chmod +x ../../scripts/find_and_replace_in_yaml_dir.sh && \ - ../../scripts/find_and_replace_in_yaml_dir.sh $(MAKEFILE_DIR)/dbt_example/airflow_dags/migration_state True False && \ + chmod +x ../../scripts/find_and_replace_in_yaml_dir.sh + ../../scripts/find_and_replace_in_yaml_dir.sh $(MAKEFILE_DIR)/dbt_example/airflow_dags/migration_state True False dagster dev -m dbt_example.dagster_defs.observe -p 3333 run_migrate: @@ -51,7 +51,7 @@ run_migrate: dagster dev -m dbt_example.dagster_defs.migrate -p 3333 wipe: ## Wipe out all the files created by the Makefile - rm -rf $$AIRFLOW_HOME $$DAGSTER_HOME + rm -rf $(AIRFLOW_HOME) $(DAGSTER_HOME) wipe_dagster: ## Wipe out all the files created by the Makefile rm -rf $$DAGSTER_HOME \ No newline at end of file diff --git a/examples/experimental/dagster-airlift/examples/dbt-example/dbt_example_tests/integration_tests/conftest.py b/examples/experimental/dagster-airlift/examples/dbt-example/dbt_example_tests/integration_tests/conftest.py index 17509bd15c386..5dea5d637d5a8 100644 --- a/examples/experimental/dagster-airlift/examples/dbt-example/dbt_example_tests/integration_tests/conftest.py +++ b/examples/experimental/dagster-airlift/examples/dbt-example/dbt_example_tests/integration_tests/conftest.py @@ -37,7 +37,7 @@ def airflow_home_fixture(local_env: None) -> Path: @pytest.fixture(name="airflow_instance") -def airflow_instance_fixture(setup: None) -> Generator[subprocess.Popen, None, None]: +def airflow_instance_fixture(local_env: None) -> Generator[subprocess.Popen, None, None]: with stand_up_airflow( airflow_cmd=["make", "run_airflow"], env=os.environ, cwd=makefile_dir() ) as process: diff --git a/examples/experimental/dagster-airlift/examples/perf-harness/Makefile b/examples/experimental/dagster-airlift/examples/perf-harness/Makefile index b073c057c21c7..8727449412e5d 100644 --- a/examples/experimental/dagster-airlift/examples/perf-harness/Makefile +++ b/examples/experimental/dagster-airlift/examples/perf-harness/Makefile @@ -14,7 +14,7 @@ help: @egrep -h '\s##\s' $(MAKEFILE_LIST) | sort | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-20s\033[0m %s\n", $$1, $$2}' dev_install: - pip install uv && \ + pip install uv uv pip install -e ../../../dagster-airlift uv pip install -e . @@ -22,10 +22,10 @@ run_airflow: airflow standalone wipe: ## Wipe out all the files created by the Makefile - rm -rf $$AIRFLOW_HOME $$DAGSTER_HOME + rm -rf $(AIRFLOW_HOME) $(DAGSTER_HOME) wipe_dagster: ## Wipe out all the files created by the Makefile - rm -rf $$DAGSTER_HOME + rm -rf $(DAGSTER_HOME) run_peer: dagster dev -m perf_harness.dagster_defs.peer @@ -48,10 +48,10 @@ run_migrate: scaffold_migrate # make airflow home and dagster home directories within current directory, set up env vars, and then # set up airflow environment. setup_local_env: scaffold_observe - make wipe && \ - mkdir -p $$AIRFLOW_HOME && \ - mkdir -p $$DAGSTER_HOME && \ - chmod +x ../../scripts/airflow_setup.sh && \ + $(MAKE) wipe + mkdir -p $$AIRFLOW_HOME + mkdir -p $$DAGSTER_HOME + chmod +x ../../scripts/airflow_setup.sh ../../scripts/airflow_setup.sh $(MAKEFILE_DIR)/perf_harness/airflow_dags run_perf_scenarios_test: diff --git a/examples/experimental/dagster-airlift/examples/perf-harness/perf_harness_tests/conftest.py b/examples/experimental/dagster-airlift/examples/perf-harness/perf_harness_tests/conftest.py index f33a6ce0bdfd4..4b25902fded2f 100644 --- a/examples/experimental/dagster-airlift/examples/perf-harness/perf_harness_tests/conftest.py +++ b/examples/experimental/dagster-airlift/examples/perf-harness/perf_harness_tests/conftest.py @@ -5,20 +5,24 @@ import pytest from dagster._core.test_utils import environ +from dagster_airlift.test.shared_fixtures import stand_up_airflow + + +def makefile_dir() -> Path: + return Path(__file__).parent.parent @pytest.fixture(name="local_env") def local_env_fixture() -> Generator[None, None, None]: - makefile_dir = Path(__file__).parent.parent - subprocess.run(["make", "setup_local_env"], cwd=makefile_dir, check=True) + subprocess.run(["make", "setup_local_env"], cwd=makefile_dir(), check=True) with environ( { - "AIRFLOW_HOME": str(makefile_dir / ".airflow_home"), - "DAGSTER_HOME": str(makefile_dir / ".dagster_home"), + "AIRFLOW_HOME": str(makefile_dir() / ".airflow_home"), + "DAGSTER_HOME": str(makefile_dir() / ".dagster_home"), } ): yield - subprocess.run(["make", "wipe"], cwd=makefile_dir, check=True) + subprocess.run(["make", "wipe"], cwd=makefile_dir(), check=True) @pytest.fixture(name="dags_dir") @@ -29,3 +33,11 @@ def dags_dir_fixture() -> Path: @pytest.fixture(name="airflow_home") def airflow_home_fixture(local_env: None) -> Path: return Path(os.environ["AIRFLOW_HOME"]) + + +@pytest.fixture(name="airflow_instance") +def airflow_instance_fixture(local_env: None) -> Generator[subprocess.Popen, None, None]: + with stand_up_airflow( + airflow_cmd=["make", "run_airflow"], env=os.environ, cwd=makefile_dir() + ) as process: + yield process