From 617635b8b29c7f12bc631497b973403a0603f5b3 Mon Sep 17 00:00:00 2001 From: lrcouto Date: Thu, 21 Mar 2024 18:32:47 -0300 Subject: [PATCH 1/8] Change pipeline test location to project root/tests Signed-off-by: lrcouto --- kedro/framework/cli/pipeline.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/kedro/framework/cli/pipeline.py b/kedro/framework/cli/pipeline.py index 9d92c6bf9e..943df87a87 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") From c0efc5d8cfe6b2e6842c4293aaf2c617bbc72433 Mon Sep 17 00:00:00 2001 From: lrcouto Date: Thu, 21 Mar 2024 19:34:54 -0300 Subject: [PATCH 2/8] Fix some test_pipeline tests Signed-off-by: lrcouto --- tests/framework/cli/pipeline/test_pipeline.py | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/tests/framework/cli/pipeline/test_pipeline.py b/tests/framework/cli/pipeline/test_pipeline.py index f452b9af6f..9b59afe6b2 100644 --- a/tests/framework/cli/pipeline/test_pipeline.py +++ b/tests/framework/cli/pipeline/test_pipeline.py @@ -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] From 3b26649ff406c13a7dcc86a1e57c29f01000f050 Mon Sep 17 00:00:00 2001 From: lrcouto Date: Fri, 22 Mar 2024 17:58:12 -0300 Subject: [PATCH 3/8] Change delete pipeline to account for new structure Signed-off-by: lrcouto --- kedro/framework/cli/pipeline.py | 7 ++++--- tests/framework/cli/micropkg/conftest.py | 2 +- tests/framework/cli/micropkg/test_micropkg_pull.py | 10 +++++----- tests/framework/cli/pipeline/conftest.py | 2 +- tests/framework/cli/pipeline/test_pipeline.py | 13 +++++++------ 5 files changed, 18 insertions(+), 16 deletions(-) diff --git a/kedro/framework/cli/pipeline.py b/kedro/framework/cli/pipeline.py index 943df87a87..4d73e2e343 100644 --- a/kedro/framework/cli/pipeline.py +++ b/kedro/framework/cli/pipeline.py @@ -313,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..52402053a5 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() @@ -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 @@ -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) @@ -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() @@ -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 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 9b59afe6b2..4e390cbf2b 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" @@ -233,8 +233,9 @@ def test_skip_copy(self, fake_repo_path, fake_project_cli, fake_metadata): assert result.exit_code == 0 assert "__init__.py': SKIPPED" in result.output + assert "test_pipeline.py': SKIPPED" in result.output assert f"parameters_{PIPELINE_NAME}.yml': SKIPPED" in result.output - assert result.output.count("SKIPPED") == 2 # only 2 files skipped + assert result.output.count("SKIPPED") == 3 # only 2 files skipped def test_failed_copy( self, fake_project_cli, fake_metadata, fake_package_path, mocker @@ -335,7 +336,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 @@ -370,7 +371,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 @@ -461,7 +462,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 @@ -501,7 +502,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 From 51764e715c64413e28acb3259aef71afd8f75252 Mon Sep 17 00:00:00 2001 From: lrcouto Date: Fri, 22 Mar 2024 19:01:39 -0300 Subject: [PATCH 4/8] Fix some tests Signed-off-by: lrcouto --- tests/framework/cli/micropkg/test_micropkg_pull.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/framework/cli/micropkg/test_micropkg_pull.py b/tests/framework/cli/micropkg/test_micropkg_pull.py index 52402053a5..a3169c0ec3 100644 --- a/tests/framework/cli/micropkg/test_micropkg_pull.py +++ b/tests/framework/cli/micropkg/test_micropkg_pull.py @@ -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 @@ -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 @@ -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" / pipeline_name config_env = env or "base" params_config = ( fake_repo_path @@ -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 @@ -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 From 1413913306bcc46065771d6bfa6f6325acd63e48 Mon Sep 17 00:00:00 2001 From: lrcouto Date: Tue, 26 Mar 2024 17:45:44 -0300 Subject: [PATCH 5/8] Change tests path on micropkg Signed-off-by: lrcouto --- kedro/framework/cli/micropkg.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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) From 20cdf15c3e2111f4773492bc236ec47649210828 Mon Sep 17 00:00:00 2001 From: lrcouto Date: Tue, 26 Mar 2024 18:14:47 -0300 Subject: [PATCH 6/8] Fix remaining tests Signed-off-by: lrcouto --- tests/framework/cli/micropkg/test_micropkg_pull.py | 2 +- tests/framework/cli/pipeline/test_pipeline.py | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/framework/cli/micropkg/test_micropkg_pull.py b/tests/framework/cli/micropkg/test_micropkg_pull.py index a3169c0ec3..5bafa9fc11 100644 --- a/tests/framework/cli/micropkg/test_micropkg_pull.py +++ b/tests/framework/cli/micropkg/test_micropkg_pull.py @@ -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 / "tests" / pipeline_name + test_dest = fake_repo_path / "tests" / "pipelines" / pipeline_name config_env = env or "base" params_config = ( fake_repo_path diff --git a/tests/framework/cli/pipeline/test_pipeline.py b/tests/framework/cli/pipeline/test_pipeline.py index 4e390cbf2b..df01d685a6 100644 --- a/tests/framework/cli/pipeline/test_pipeline.py +++ b/tests/framework/cli/pipeline/test_pipeline.py @@ -233,9 +233,8 @@ def test_skip_copy(self, fake_repo_path, fake_project_cli, fake_metadata): assert result.exit_code == 0 assert "__init__.py': SKIPPED" in result.output - assert "test_pipeline.py': SKIPPED" in result.output assert f"parameters_{PIPELINE_NAME}.yml': SKIPPED" in result.output - assert result.output.count("SKIPPED") == 3 # only 2 files skipped + assert result.output.count("SKIPPED") == 2 # only 2 files skipped def test_failed_copy( self, fake_project_cli, fake_metadata, fake_package_path, mocker From 0d7db88c6fca47a56ede27aa79c875ab9ed24c46 Mon Sep 17 00:00:00 2001 From: lrcouto Date: Tue, 26 Mar 2024 18:39:42 -0300 Subject: [PATCH 7/8] Add changes to release notes Signed-off-by: lrcouto --- RELEASE.md | 1 + 1 file changed, 1 insertion(+) diff --git a/RELEASE.md b/RELEASE.md index d50e1b936d..a43057c1da 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -11,6 +11,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/`. ## Breaking changes to the API * Methods `_is_project` and `_find_kedro_project` have been moved to `kedro.utils`. We recommend not using private methods in your code, but if you do, please update your code to use the new location. From 73b235a0303ab1f693f7194ae882354bb4b12d18 Mon Sep 17 00:00:00 2001 From: lrcouto Date: Wed, 27 Mar 2024 10:55:47 -0300 Subject: [PATCH 8/8] Update file structure on micropackaging doc page Signed-off-by: lrcouto --- docs/source/nodes_and_pipelines/micro_packaging.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) 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`.