Change pipeline test location to project root/tests (#3731)

* Change pipeline test location to project root/tests Signed-off-by: lrcouto <laurarccouto@gmail.com> * Fix some test_pipeline tests Signed-off-by: lrcouto <laurarccouto@gmail.com> * Change delete pipeline to account for new structure Signed-off-by: lrcouto <laurarccouto@gmail.com> * Fix some tests Signed-off-by: lrcouto <laurarccouto@gmail.com> * Change tests path on micropkg Signed-off-by: lrcouto <laurarccouto@gmail.com> * Fix remaining tests Signed-off-by: lrcouto <laurarccouto@gmail.com> * Add changes to release notes Signed-off-by: lrcouto <laurarccouto@gmail.com> * Update file structure on micropackaging doc page Signed-off-by: lrcouto <laurarccouto@gmail.com> --------- Signed-off-by: lrcouto <laurarccouto@gmail.com> Signed-off-by: L. R. Couto <57910428+lrcouto@users.noreply.github.com>
kedro-org · Apr 3, 2024 · 5e9f7c2 · 5e9f7c2
1 parent 00789fa
commit 5e9f7c2
Show file tree

Hide file tree

Showing 8 changed files with 37 additions and 39 deletions.
diff --git a/RELEASE.md b/RELEASE.md
@@ -13,6 +13,7 @@
 ## Bug fixes and other changes
 * Updated `kedro pipeline create` and `kedro pipeline delete` to read the base environment from the project settings.
 * Updated CLI command `kedro catalog resolve` to read credentials properly.
+* Changed the path of where pipeline tests generated with `kedro pipeline create` from `<project root>/src/tests/pipelines/<pipeline name>` to `<project root>/tests/pipelines/<pipeline name>`.
 * Updated ``.gitignore`` to prevent pushing Mlflow local runs folder to a remote forge when using mlflow and git.
 
 ## Breaking changes to the API

diff --git a/docs/source/nodes_and_pipelines/micro_packaging.md b/docs/source/nodes_and_pipelines/micro_packaging.md
@@ -20,15 +20,15 @@ When you package your micro-package, such as a modular pipeline for example, Ked
 ├── conf
 │   └── base
 │       └── parameters_{{pipeline_name*}}  <-- All parameter file(s)
+├── tests
+│   ├── init__.py
+│   └── pipelines
+│       └── {{pipeline_name}}              <-- Pipeline tests
 └── src
-    ├── my_project
-    │   ├── __init__.py
-    │   └── pipelines
-    │       └── {{pipeline_name}}    <-- Pipeline folder
-    └── tests
+    └── my_project
         ├── __init__.py
         └── pipelines
-            └── {{pipeline_name}}    <-- Pipeline tests
+            └── {{pipeline_name}}          <-- Pipeline folder
 ```
 
 Kedro will also include any requirements found in `src/<package_name>/pipelines/<micropkg_name>/requirements.txt` in the micro-package tar file. These requirements will later be taken into account when pulling a micro-package via `kedro micropkg pull`.

diff --git a/kedro/framework/cli/micropkg.py b/kedro/framework/cli/micropkg.py
@@ -805,7 +805,7 @@ def _move_package_with_conflicting_name(
 
     # Copy tests in appropriate folder structure
     if tests_path.exists():
-        tests_target = tests_path.relative_to(project_metadata.source_dir)
+        tests_target = tests_path.relative_to(project_metadata.project_path)
         full_path = _create_nested_package(project, tests_target)
         # overwrite=True to update the __init__.py files generated by create_package
         _sync_dirs(tests_path, full_path, overwrite=True)

diff --git a/kedro/framework/cli/pipeline.py b/kedro/framework/cli/pipeline.py
@@ -108,6 +108,7 @@ def create_pipeline(
 ) -> None:  # noqa: unused-argument
     """Create a new modular pipeline by providing a name."""
     package_dir = metadata.source_dir / metadata.package_name
+    project_root = metadata.project_path / metadata.project_name
     conf_source = settings.CONF_SOURCE
     project_conf_path = metadata.project_path / conf_source
     base_env = settings.CONFIG_LOADER_ARGS.get("base_env", "base")
@@ -131,7 +132,7 @@ def create_pipeline(
     click.secho(f"Using pipeline template at: '{template_path}'")
 
     result_path = _create_pipeline(name, template_path, package_dir / "pipelines")
-    _copy_pipeline_tests(name, result_path, package_dir)
+    _copy_pipeline_tests(name, result_path, project_root)
     _copy_pipeline_configs(result_path, project_conf_path, skip_config, env=env)
     click.secho(f"\nPipeline '{name}' was successfully created.\n", fg="green")
 
@@ -312,20 +313,21 @@ def _get_artifacts_to_package(
 ) -> tuple[Path, Path, Path]:
     """From existing project, returns in order: source_path, tests_path, config_paths"""
     package_dir = project_metadata.source_dir / project_metadata.package_name
+    project_root = project_metadata.project_path
     project_conf_path = project_metadata.project_path / settings.CONF_SOURCE
     artifacts = (
         Path(package_dir, *module_path.split(".")),
-        Path(package_dir.parent, "tests", *module_path.split(".")),
+        Path(project_root, "tests", *module_path.split(".")),
         project_conf_path / env,
     )
     return artifacts
 
 
 def _copy_pipeline_tests(
-    pipeline_name: str, result_path: Path, package_dir: Path
+    pipeline_name: str, result_path: Path, project_root: Path
 ) -> None:
     tests_source = result_path / "tests"
-    tests_target = package_dir.parent / "tests" / "pipelines" / pipeline_name
+    tests_target = project_root.parent / "tests" / "pipelines" / pipeline_name
     try:
         _sync_dirs(tests_source, tests_target)
     finally:

diff --git a/tests/framework/cli/micropkg/conftest.py b/tests/framework/cli/micropkg/conftest.py
@@ -53,7 +53,7 @@ def cleanup_pipelines(fake_repo_path, fake_package_path):
             if each.is_file():
                 each.unlink()
 
-        tests = fake_repo_path / "src" / "tests" / "pipelines" / pipeline
+        tests = fake_repo_path / "tests" / "pipelines" / pipeline
         if tests.is_dir():
             shutil.rmtree(str(tests))
 

diff --git a/tests/framework/cli/micropkg/test_micropkg_pull.py b/tests/framework/cli/micropkg/test_micropkg_pull.py
@@ -82,7 +82,7 @@ def test_pull_local_sdist(
         config_path = (
             fake_repo_path / settings.CONF_SOURCE / "base" / "pipelines" / PIPELINE_NAME
         )
-        test_path = fake_repo_path / "src" / "tests" / "pipelines" / PIPELINE_NAME
+        test_path = fake_repo_path / "tests" / "pipelines" / PIPELINE_NAME
         # Make sure the files actually deleted before pulling from the sdist file.
         assert not source_path.exists()
         assert not test_path.exists()
@@ -107,7 +107,7 @@ def test_pull_local_sdist(
         pipeline_name = alias or PIPELINE_NAME
         destination = destination or Path()
         source_dest = fake_package_path / destination / pipeline_name
-        test_dest = fake_repo_path / "src" / "tests" / destination / pipeline_name
+        test_dest = fake_repo_path / "tests" / destination / pipeline_name
         config_env = env or "base"
         params_config = (
             fake_repo_path
@@ -151,7 +151,7 @@ def test_pull_local_sdist_compare(
         call_micropkg_package(fake_project_cli, fake_metadata, alias=pipeline_name)
 
         source_path = fake_package_path / "pipelines" / PIPELINE_NAME
-        test_path = fake_repo_path / "src" / "tests" / "pipelines" / PIPELINE_NAME
+        test_path = fake_repo_path / "tests" / "pipelines" / PIPELINE_NAME
         source_params_config = (
             fake_repo_path
             / settings.CONF_SOURCE
@@ -178,7 +178,7 @@ def test_pull_local_sdist_compare(
         pipeline_name = alias or pipeline_name
         destination = destination or Path()
         source_dest = fake_package_path / destination / pipeline_name
-        test_dest = fake_repo_path / "src" / "tests" / destination / pipeline_name
+        test_dest = fake_repo_path / "tests" / destination / pipeline_name
         config_env = env or "base"
         dest_params_config = (
             fake_repo_path
@@ -225,7 +225,7 @@ def test_micropkg_pull_same_alias_package_name(
         assert "pulled and unpacked" in result.output
 
         source_dest = fake_package_path / destination / pipeline_name
-        test_dest = fake_repo_path / "src" / "tests" / destination / pipeline_name
+        test_dest = fake_repo_path / "tests" / destination / pipeline_name
         config_env = "base"
         params_config = (
             fake_repo_path
@@ -274,7 +274,7 @@ def test_micropkg_pull_nested_destination(
         assert "pulled and unpacked" in result.output
 
         source_dest = fake_package_path / destination / pipeline_name
-        test_dest = fake_repo_path / "src" / "tests" / destination / pipeline_name
+        test_dest = fake_repo_path / "tests" / destination / pipeline_name
         config_env = "base"
         params_config = (
             fake_repo_path
@@ -431,7 +431,7 @@ def test_pull_tests_missing(
         but `tests` directory is missing from the sdist file.
         """
         call_pipeline_create(fake_project_cli, fake_metadata)
-        test_path = fake_repo_path / "src" / "tests" / "pipelines" / PIPELINE_NAME
+        test_path = fake_repo_path / "tests" / "pipelines" / PIPELINE_NAME
         shutil.rmtree(test_path)
         assert not test_path.exists()
         call_micropkg_package(fake_project_cli, fake_metadata)
@@ -464,7 +464,7 @@ def test_pull_tests_missing(
 
         pipeline_name = alias or PIPELINE_NAME
         source_dest = fake_package_path / pipeline_name
-        test_dest = fake_repo_path / "src" / "tests" / pipeline_name
+        test_dest = fake_repo_path / "tests" / "pipelines" / pipeline_name
         config_env = env or "base"
         params_config = (
             fake_repo_path
@@ -504,7 +504,7 @@ def test_pull_config_missing(
         call_pipeline_delete(fake_project_cli, fake_metadata)
 
         source_path = fake_package_path / "pipelines" / PIPELINE_NAME
-        test_path = fake_repo_path / "src" / "tests" / "pipelines" / PIPELINE_NAME
+        test_path = fake_repo_path / "tests" / "pipelines" / PIPELINE_NAME
         # Make sure the files actually deleted before pulling from the sdist file.
         assert not source_path.exists()
         assert not test_path.exists()
@@ -525,7 +525,7 @@ def test_pull_config_missing(
 
         pipeline_name = alias or PIPELINE_NAME
         source_dest = fake_package_path / pipeline_name
-        test_dest = fake_repo_path / "src" / "tests" / pipeline_name
+        test_dest = fake_repo_path / "tests" / pipeline_name
         config_env = env or "base"
         dest_params_config = (
             fake_repo_path
@@ -566,7 +566,7 @@ def test_pull_from_pypi(
         call_pipeline_delete(fake_project_cli, fake_metadata)
 
         source_path = fake_package_path / "pipelines" / PIPELINE_NAME
-        test_path = fake_repo_path / "src" / "tests" / "pipelines" / PIPELINE_NAME
+        test_path = fake_repo_path / "tests" / "pipelines" / PIPELINE_NAME
         source_params_config = (
             fake_repo_path
             / settings.CONF_SOURCE
@@ -623,7 +623,7 @@ def get_all(self, name, failobj=None):
 
         pipeline_name = alias or PIPELINE_NAME
         source_dest = fake_package_path / pipeline_name
-        test_dest = fake_repo_path / "src" / "tests" / pipeline_name
+        test_dest = fake_repo_path / "tests" / pipeline_name
         config_env = env or "base"
         dest_params_config = (
             fake_repo_path

diff --git a/tests/framework/cli/pipeline/conftest.py b/tests/framework/cli/pipeline/conftest.py
@@ -72,7 +72,7 @@ def cleanup_pipelines(fake_repo_path, fake_package_path):
                 if dirpath.is_dir() and not any(dirpath.iterdir()):
                     dirpath.rmdir()
 
-        tests = fake_repo_path / "src" / "tests" / "pipelines" / pipeline
+        tests = fake_repo_path / "tests" / "pipelines" / pipeline
         if tests.is_dir():
             shutil.rmtree(str(tests))
 

diff --git a/tests/framework/cli/pipeline/test_pipeline.py b/tests/framework/cli/pipeline/test_pipeline.py
@@ -18,7 +18,7 @@
 @pytest.fixture(params=["base"])
 def make_pipelines(request, fake_repo_path, fake_package_path, mocker):
     source_path = fake_package_path / "pipelines" / PIPELINE_NAME
-    tests_path = fake_repo_path / "src" / "tests" / "pipelines" / PIPELINE_NAME
+    tests_path = fake_repo_path / "tests" / "pipelines" / PIPELINE_NAME
     conf_path = fake_repo_path / settings.CONF_SOURCE / request.param
     # old conf structure for 'pipeline delete' command backward compatibility
     old_conf_path = conf_path / "parameters"
@@ -74,7 +74,7 @@ def test_create_pipeline(
         assert actual_configs == expected_configs
 
         # tests
-        test_dir = fake_repo_path / "src" / "tests" / "pipelines" / PIPELINE_NAME
+        test_dir = fake_repo_path / "tests" / "pipelines" / PIPELINE_NAME
         expected_files = {"__init__.py", "test_pipeline.py"}
         actual_files = {f.name for f in test_dir.iterdir()}
         assert actual_files == expected_files
@@ -168,7 +168,7 @@ def test_create_pipeline_skip_config(
         conf_dirs = list((fake_repo_path / settings.CONF_SOURCE).rglob(PIPELINE_NAME))
         assert not conf_dirs  # no configs created for the pipeline
 
-        test_dir = fake_repo_path / "src" / "tests" / "pipelines" / PIPELINE_NAME
+        test_dir = fake_repo_path / "tests" / "pipelines" / PIPELINE_NAME
         assert test_dir.is_dir()
 
     def test_catalog_and_params(
@@ -223,14 +223,9 @@ def test_skip_copy(self, fake_repo_path, fake_project_cli, fake_metadata):
 
         # create __init__.py in tests
         tests_init = (
-            fake_repo_path
-            / "src"
-            / "tests"
-            / "pipelines"
-            / PIPELINE_NAME
-            / "__init__.py"
+            fake_repo_path / "tests" / "pipelines" / PIPELINE_NAME / "__init__.py"
         )
-        tests_init.parent.mkdir(parents=True)
+        tests_init.parent.mkdir(parents=True, exist_ok=True)
         tests_init.touch()
 
         cmd = ["pipeline", "create", PIPELINE_NAME]
@@ -340,7 +335,7 @@ def test_delete_pipeline(
         )
 
         source_path = fake_package_path / "pipelines" / PIPELINE_NAME
-        tests_path = fake_repo_path / "src" / "tests" / "pipelines" / PIPELINE_NAME
+        tests_path = fake_repo_path / "tests" / "pipelines" / PIPELINE_NAME
         conf_path = fake_repo_path / settings.CONF_SOURCE / expected_conf
         params_path = conf_path / f"parameters_{PIPELINE_NAME}.yml"
         # old params structure for 'pipeline delete' command backward compatibility
@@ -375,7 +370,7 @@ def test_delete_pipeline_skip(
             ["pipeline", "delete", "-y", PIPELINE_NAME],
             obj=fake_metadata,
         )
-        tests_path = fake_repo_path / "src" / "tests" / "pipelines" / PIPELINE_NAME
+        tests_path = fake_repo_path / "tests" / "pipelines" / PIPELINE_NAME
         params_path = (
             fake_repo_path
             / settings.CONF_SOURCE
@@ -466,7 +461,7 @@ def test_pipeline_delete_confirmation(
         )
 
         source_path = fake_package_path / "pipelines" / PIPELINE_NAME
-        tests_path = fake_repo_path / "src" / "tests" / "pipelines" / PIPELINE_NAME
+        tests_path = fake_repo_path / "tests" / "pipelines" / PIPELINE_NAME
         params_path = (
             fake_repo_path
             / settings.CONF_SOURCE
@@ -506,7 +501,7 @@ def test_pipeline_delete_confirmation_skip(
             obj=fake_metadata,
         )
 
-        tests_path = fake_repo_path / "src" / "tests" / "pipelines" / PIPELINE_NAME
+        tests_path = fake_repo_path / "tests" / "pipelines" / PIPELINE_NAME
         params_path = (
             fake_repo_path
             / settings.CONF_SOURCE