From 5e9f7c22ad630400ee062c0b3675b2243e742844 Mon Sep 17 00:00:00 2001 From: "L. R. Couto" <57910428+lrcouto@users.noreply.github.com> Date: Wed, 3 Apr 2024 16:58:47 -0300 Subject: [PATCH] Change pipeline test location to project root/tests (#3731) * Change pipeline test location to project root/tests Signed-off-by: lrcouto * Fix some test_pipeline tests Signed-off-by: lrcouto * Change delete pipeline to account for new structure Signed-off-by: lrcouto * Fix some tests Signed-off-by: lrcouto * Change tests path on micropkg Signed-off-by: lrcouto * Fix remaining tests Signed-off-by: lrcouto * Add changes to release notes Signed-off-by: lrcouto * Update file structure on micropackaging doc page Signed-off-by: lrcouto --------- Signed-off-by: lrcouto Signed-off-by: L. R. Couto <57910428+lrcouto@users.noreply.github.com> --- RELEASE.md | 1 + .../nodes_and_pipelines/micro_packaging.md | 12 +++++----- kedro/framework/cli/micropkg.py | 2 +- kedro/framework/cli/pipeline.py | 10 ++++---- tests/framework/cli/micropkg/conftest.py | 2 +- .../cli/micropkg/test_micropkg_pull.py | 24 +++++++++---------- tests/framework/cli/pipeline/conftest.py | 2 +- tests/framework/cli/pipeline/test_pipeline.py | 23 +++++++----------- 8 files changed, 37 insertions(+), 39 deletions(-) diff --git a/RELEASE.md b/RELEASE.md index 3086581aa4..dfe9e6fe8e 100644 --- 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 `/src/tests/pipelines/` to `/tests/pipelines/`. * 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 index cd9df75252..bfb4e7c37f 100644 --- 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//pipelines//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 index 2365f9f0eb..46981e10b1 100644 --- 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 index 9d92c6bf9e..4d73e2e343 100644 --- 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 index 5d241f9488..af6cd11f40 100644 --- 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 index c0033a004b..5bafa9fc11 100644 --- 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 index f192ae7e3b..a6f326abe9 100644 --- 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 index f452b9af6f..df01d685a6 100644 --- 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