diff --git a/cou/apps/base.py b/cou/apps/base.py index ade6a05b..06a883e6 100644 --- a/cou/apps/base.py +++ b/cou/apps/base.py @@ -499,18 +499,16 @@ def generate_upgrade_plan(self, target: OpenStackRelease) -> ApplicationUpgradeP :return: Full upgrade plan if the Application is able to generate it. :rtype: ApplicationUpgradePlan """ - upgrade_steps = ApplicationUpgradePlan( + upgrade_plan = ApplicationUpgradePlan( description=f"Upgrade plan for '{self.name}' to {target}", ) - all_steps = ( - self.pre_upgrade_steps(target) - + self.upgrade_steps(target) - + self.post_upgrade_steps(target) - ) - for step in all_steps: - if step: - upgrade_steps.add_step(step) - return upgrade_steps + upgrade_plan.sub_steps = [ + *self.pre_upgrade_steps(target), + *self.upgrade_steps(target), + *self.post_upgrade_steps(target), + ] + + return upgrade_plan def _get_upgrade_current_release_packages_step(self) -> PreUpgradeStep: """Get step for upgrading software packages to the latest of the current release. diff --git a/cou/steps/__init__.py b/cou/steps/__init__.py index 9d0f4a3c..fa2c7d73 100644 --- a/cou/steps/__init__.py +++ b/cou/steps/__init__.py @@ -168,6 +168,7 @@ def description(self, description: str) -> None: """ if not description and self._coro: raise ValueError("Every coroutine should have a description") + self._description = description @property @@ -224,6 +225,10 @@ def add_step(self, step: BaseStep) -> None: if not isinstance(step, BaseStep): raise TypeError("Cannot add an upgrade step that is not derived from BaseStep") + if not step: + logger.debug("skipping adding empty step") + return + self._sub_steps.append(step) def cancel(self, safe: bool = True) -> None: diff --git a/tests/unit/steps/test_steps.py b/tests/unit/steps/test_steps.py index 02cc36d6..93b7b3ea 100644 --- a/tests/unit/steps/test_steps.py +++ b/tests/unit/steps/test_steps.py @@ -109,12 +109,14 @@ def test_step_bool(): sub_step = BaseStep(description="a.a") assert bool(sub_step) is False - plan.sub_steps = [sub_step] + plan.add_step(sub_step) assert bool(plan) is False # coroutine in the plan sub_steps tree sub_sub_step = BaseStep(description="a.a.a", coro=mock_coro("a.a.a")) - sub_step.sub_steps = [sub_sub_step] + sub_step.add_step(sub_sub_step) + plan.add_step(sub_step) + assert bool(sub_sub_step) is True assert bool(sub_step) is True assert bool(plan) is True @@ -202,6 +204,16 @@ def test_step_add_step(): exp_sub_steps = 3 plan = BaseStep(description="plan") for i in range(exp_sub_steps): + plan.add_step(BaseStep(description=f"sub-step-{i}", coro=mock_coro())) + + assert len(plan.sub_steps) == exp_sub_steps + + +def test_step_add_step_skipping_empty(): + """Test BaseStep skipping to add empty sub steps.""" + exp_sub_steps = 0 + plan = BaseStep(description="plan") + for i in range(3): plan.add_step(BaseStep(description=f"sub-step-{i}")) assert len(plan.sub_steps) == exp_sub_steps @@ -219,9 +231,13 @@ def test_step_add_step_failed(): def test_step_cancel_safe(): """Test step safe cancel.""" plan = BaseStep(description="plan") - plan.sub_steps = sub_steps = [BaseStep(description=f"sub-{i}") for i in range(10)] + plan.sub_steps = sub_steps = [ + BaseStep(description=f"sub-{i}", coro=mock_coro()) for i in range(10) + ] # add sub-sub-steps to one sub-step - sub_steps[0].sub_steps = [BaseStep(description=f"sub-0.{i}") for i in range(3)] + sub_steps[0].sub_steps = [ + BaseStep(description=f"sub-0.{i}", coro=mock_coro()) for i in range(3) + ] plan.cancel()