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

fix(configuration_support): raise explicit error on conditional steps addition #819

Merged
merged 4 commits into from
Aug 25, 2023
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
63 changes: 56 additions & 7 deletions tests/test_conditional_steps.py
Original file line number Diff line number Diff line change
Expand Up @@ -223,17 +223,66 @@ def test_prebuild_clean(test_env: ConditionalStepsTestEnv,
test_env.check_fail(steps_info)


def test_conditional_step_critical(test_env: ConditionalStepsTestEnv) -> None:
steps_info: StepsInfo = test_env.build_conditional_steps_info(is_conditional_step_passed=False)
steps_info.conditional_step.critical = True
test_env.check_success(steps_info)
assert test_env.captured_out.out
assert "WARNING" in test_env.captured_out.out


# TODO: implement support of conditional step with children
# https://github.com/Samsung/Universum/issues/709
def test_conditional_step_with_children(test_env: ConditionalStepsTestEnv) -> None:
def test_conditional_step_with_children(test_env: ConditionalStepsTestEnv, capsys: pytest.CaptureFixture) -> None:
steps_info: StepsInfo = test_env.build_conditional_steps_info(is_conditional_step_passed=True)
steps_info.conditional_step.children = Configuration([Step(name=" dummy child 1"), Step(name=" dummy child 2")])
test_env.check_fail(steps_info)

captured_out = capsys.readouterr()
assert "not supported" in captured_out.out

def test_conditional_step_critical(test_env: ConditionalStepsTestEnv) -> None:
steps_info: StepsInfo = test_env.build_conditional_steps_info(is_conditional_step_passed=False)
steps_info.conditional_step.critical = True
test_env.check_success(steps_info)
assert test_env.captured_out.out
assert "WARNING" in test_env.captured_out.out

# TODO: implement support of conditional steps addition and multiplication
k-dovgan marked this conversation as resolved.
Show resolved Hide resolved
# https://github.com/Samsung/Universum/issues/709
def test_add_step_to_conditional(test_env: ConditionalStepsTestEnv, capsys: pytest.CaptureFixture) -> None:
config_lines: List[str] = [
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to store config as a list of lines instead of multiline text variable?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Made similar to the _write_config_file() for uniformity.

"from universum.configuration_support import Configuration, Step",
"true_branch_config = Configuration([Step(name='true branch')])",
"conditional_step = Step(name='conditional', if_succeeded=true_branch_config)",
"another_step = Step(name='another step')",
"configs = Configuration([conditional_step + another_step])"
]
config: str = "\n".join(config_lines)
test_env.configs_file.write_text(config, "utf-8")
test_env.run(expect_failure=True)

captured_out = capsys.readouterr()
k-dovgan marked this conversation as resolved.
Show resolved Hide resolved
assert "not supported" in captured_out.out


# TODO: during implementation of the full conditional steps addition support, refactor this test to avoid direct usage
# of the ConditionalStepsTestEnv "internal" API (underscored methods) and config building duplication
# https://github.com/Samsung/Universum/issues/709
def test_add_conditional_to_step(test_env: ConditionalStepsTestEnv, capsys: pytest.CaptureFixture) -> None:
# pylint: disable = protected-access
steps_info = test_env.build_conditional_steps_info(is_conditional_step_passed=True)
true_branch_config = test_env._build_configuration_string(steps_info.true_branch_steps)
miltolstoy marked this conversation as resolved.
Show resolved Hide resolved
config_lines: List[str] = [
"from universum.configuration_support import Configuration, Step",
f"true_branch_config = {true_branch_config}",
f"conditional_step = Step(**{str(steps_info.conditional_step)})",
"another_step = Step(name='another step')",

# `true/false_branch_steps` should be Python objects from this script
"conditional_step.is_conditional = True",
"conditional_step.if_succeeded = true_branch_config",

"configs = Configuration([another_step + conditional_step])"
]
config: str = "\n".join(config_lines)
test_env.configs_file.write_text(config, "utf-8")
test_env.run()

test_env._check_conditional_step_artifacts_present(steps_info)
miltolstoy marked this conversation as resolved.
Show resolved Hide resolved
test_env._check_executed_step_artifacts_present(steps_info)
miltolstoy marked this conversation as resolved.
Show resolved Hide resolved
# pylint: enable = protected-access
4 changes: 4 additions & 0 deletions universum/configuration_support.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from warnings import warn
import copy
import os
from .lib.ci_exception import CriticalCiException


__all__ = [
Expand Down Expand Up @@ -397,6 +398,9 @@ def __add__(self, other: 'Step') -> 'Step':
>>> step2 + step1
{'name': 'barfoo', 'command': ['bar', 'foo'], 'critical': True, 'background': True, 'my_var1': 'barfoo', 'my_var2': 'baz'}
"""
if self.is_conditional:
i-keliukh marked this conversation as resolved.
Show resolved Hide resolved
# TODO: https://github.com/Samsung/Universum/issues/709
raise CriticalCiException("Conditional steps addition is not supported yet")
return Step(
name=self.name + other.name,
command=self.command + other.command,
Expand Down
2 changes: 1 addition & 1 deletion universum/modules/launcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,7 @@ def process_project_configs(self) -> configuration_support.Configuration:
raise CriticalCiException(text)

if self._is_conditional_step_with_children_present(self.project_config):
raise CriticalCiException("Conditional step doesn't support children configuration")
raise CriticalCiException("Conditional steps with child configurations are not supported")
self._warn_if_critical_conditional_steps_present(self.project_config)

return self.project_config
Expand Down
Loading