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

[5/n][pipeline-gen] Helper method to determine whether test should automatically run #41

Closed
wants to merge 2 commits into from
Closed
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
14 changes: 14 additions & 0 deletions scripts/pipeline_generator/pipeline_generator_helper.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
from typing import List

from .step import TestStep

def step_should_run(step: TestStep, run_all: bool, list_file_diff: List[str]) -> bool:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where do these input args come from? I feel that without the context, this function (and the test) does not really serve much purpose.

you should put a more meaningful TestSteps into context, and test with that.

like there are many other fields in TestStep? why does this function need to send the entire TestStep in together? why is this not a member function of TestStep but a standalone function? why run_all does not really "run all", but will skip the optional ones?

"""Determine whether the step should automatically run or not."""
if step.optional:
return False
if not step.source_file_dependencies or run_all:
return True
return any(source_file in diff_file
for source_file in step.source_file_dependencies
for diff_file in list_file_diff
)
50 changes: 50 additions & 0 deletions scripts/tests/pipeline_generator/test_pipeline_generator_helper.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import pytest
import sys

from scripts.pipeline_generator.pipeline_generator_helper import step_should_run
from scripts.pipeline_generator.step import TestStep

def _get_test_step():
return TestStep(
label="Test",
command="echo 'Hello, World!'",
)

@pytest.mark.parametrize(
("run_all, source_file_dependencies, list_file_diff, expected"),
[
(False, None, [], True),
(True, None, [], True),
(False, ["file1", "file2"], [], False),
(True, ["file1", "file2"], [], True),
(False, ["file1", "file2"], ["file1"], True),
(False, ["file1", "file2"], ["file3"], False),
(True, ["file1", "file2"], ["file3"], True),
]
)
def test_step_should_run(run_all, source_file_dependencies, list_file_diff, expected):
test_step = _get_test_step()
test_step.source_file_dependencies = source_file_dependencies
assert step_should_run(test_step, run_all, list_file_diff) == expected

@pytest.mark.parametrize(
("run_all, source_file_dependencies, list_file_diff"),
[
(False, None, []),
(True, None, []),
(False, ["file1", "file2"], []),
(True, ["file1", "file2"], []),
(False, ["file1", "file2"], ["file1"]),
(False, ["file1", "file2"], ["file3"]),
(True, ["file1", "file2"], ["file3"]),
]
)
def test_step_should_run_optional(run_all, source_file_dependencies, list_file_diff):
test_step = _get_test_step()
test_step.optional = True # When optional is True, step should not run at all
test_step.source_file_dependencies = source_file_dependencies

assert step_should_run(test_step, run_all, list_file_diff) == False

if __name__ == "__main__":
sys.exit(pytest.main(["-v", __file__]))
Loading