Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change pipeline test location to project root/tests #3731

Merged
merged 15 commits into from
Apr 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions RELEASE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 6 additions & 6 deletions docs/source/nodes_and_pipelines/micro_packaging.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
2 changes: 1 addition & 1 deletion kedro/framework/cli/micropkg.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
10 changes: 6 additions & 4 deletions kedro/framework/cli/pipeline.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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")

Expand Down Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion tests/framework/cli/micropkg/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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))

Expand Down
24 changes: 12 additions & 12 deletions tests/framework/cli/micropkg/test_micropkg_pull.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion tests/framework/cli/pipeline/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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))

Expand Down
23 changes: 9 additions & 14 deletions tests/framework/cli/pipeline/test_pipeline.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down