[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.
dagster-io · Sep 19, 2024 · 6fdd8d4 · 6fdd8d4
1 parent 1ed2453
commit 6fdd8d4
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 24 deletions.
diff --git 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
diff --git a/...ntal/dagster-airlift/examples/dbt-example/dbt_example_tests/integration_tests/conftest.py b/...ntal/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
@@ -14,18 +14,18 @@ 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 .
 
 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
@@ -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