From 330b8720b392f7c443b44ca10dfca9724d5dff3a Mon Sep 17 00:00:00 2001 From: "jean-francois.figue" Date: Fri, 14 Feb 2025 17:55:29 +0100 Subject: [PATCH 1/6] test: add 2 new tests, cf #1457 --- .../wrappers/test_retry_discipline.py | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/tests/disciplines/wrappers/test_retry_discipline.py b/tests/disciplines/wrappers/test_retry_discipline.py index 7af07c2e9d..d6050a2251 100644 --- a/tests/disciplines/wrappers/test_retry_discipline.py +++ b/tests/disciplines/wrappers/test_retry_discipline.py @@ -26,6 +26,7 @@ from numpy import array from gemseo import create_discipline from gemseo.core.discipline import Discipline +from gemseo.core.execution_status import ExecutionStatus from gemseo.disciplines.wrappers.retry_discipline import RetryDiscipline from gemseo.utils.timer import Timer @@ -57,6 +58,11 @@ def a_long_time_running_discipline() -> Discipline: return DisciplineLongTimeRunning() +@pytest.fixture +def a_fictive_discipline() -> Discipline: + return FictiveDiscipline() + + class CrashingDisciplineInRun(Discipline): """A discipline raising NotImplementedError in ``_run``.""" @@ -72,6 +78,25 @@ class DisciplineLongTimeRunning(Discipline): time.sleep(5.0) +class FictiveDiscipline(Discipline): + """Discipline to be executed several times. + + - The first 2 times, raise a RuntimeError, + - and finally succeed. + """ + + def __init__(self) -> None: + super().__init__() + self.attempt = 0 + + def _run(self, input_data: StrKeyMapping) -> StrKeyMapping: + self.attempt += 1 + if self.attempt < 3: + msg = "runtime error in FictiveDiscipline" + raise RuntimeError(msg) + return {} + + @pytest.mark.parametrize("timeout", [math.inf, 10.0]) def test_retry_discipline(an_analytic_discipline, timeout, caplog) -> None: """Test discipline, no timeout set.""" @@ -232,3 +257,38 @@ def test_retry_discipline_timeout_feature( "Failed to execute discipline DisciplineLongTimeRunning after 1 attempt." ) assert log_message in caplog.text + + +@pytest.mark.parametrize("n_trials", [1, 3]) +def test_1_3times_failing(a_crashing_analytic_discipline, n_trials, caplog) -> None: + """Test a discipline crashing each time, n_trials = 1 or 3.""" + disc = RetryDiscipline( + a_crashing_analytic_discipline, + n_trials=n_trials, + ) + with pytest.raises(ZeroDivisionError, match="float division by zero"): + disc.execute({"x": array([0.0])}) + + assert disc.n_executions == n_trials + assert disc.io.data == {"x": array([0.0])} + + assert disc.execution_status.value == ExecutionStatus.Status.FAILED + + plural_suffix = "s" if n_trials > 1 else "" + log_message = ( + "Failed to execute discipline AnalyticDiscipline" + f" after {n_trials} attempt{plural_suffix}." + ) + assert log_message in caplog.text + + +def test_2fails_then_succeed(a_fictive_discipline, caplog) -> None: + """Test a discipline crashing the 2 first times, then succeeding.""" + disc = RetryDiscipline( + a_fictive_discipline, + n_trials=3, + ) + disc.execute() + assert disc.n_executions == 3 + assert disc.io.data == {} + assert disc.execution_status.value == ExecutionStatus.Status.DONE -- GitLab From 994fe4ea16b29776af0378e8f88a36d1a7e312f7 Mon Sep 17 00:00:00 2001 From: "jean-francois.figue" Date: Thu, 20 Feb 2025 11:12:18 +0100 Subject: [PATCH 2/6] test: remove keyword fixture. Set job DONE before time.sleep --- src/gemseo/disciplines/wrappers/retry_discipline.py | 2 +- tests/disciplines/wrappers/test_retry_discipline.py | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/gemseo/disciplines/wrappers/retry_discipline.py b/src/gemseo/disciplines/wrappers/retry_discipline.py index aff19faac1..aad99b9e1d 100644 --- a/src/gemseo/disciplines/wrappers/retry_discipline.py +++ b/src/gemseo/disciplines/wrappers/retry_discipline.py @@ -157,8 +157,8 @@ class RetryDiscipline(Discipline): raise current_error = error - time.sleep(self.wait_time) self.__discipline.execution_status.value = ExecutionStatus.Status.DONE + time.sleep(self.wait_time) plural_suffix = "s" if self.n_trials > 1 else "" LOGGER.error( diff --git a/tests/disciplines/wrappers/test_retry_discipline.py b/tests/disciplines/wrappers/test_retry_discipline.py index d6050a2251..0b6c3df36d 100644 --- a/tests/disciplines/wrappers/test_retry_discipline.py +++ b/tests/disciplines/wrappers/test_retry_discipline.py @@ -58,7 +58,6 @@ def a_long_time_running_discipline() -> Discipline: return DisciplineLongTimeRunning() -@pytest.fixture def a_fictive_discipline() -> Discipline: return FictiveDiscipline() @@ -282,10 +281,10 @@ def test_1_3times_failing(a_crashing_analytic_discipline, n_trials, caplog) -> N assert log_message in caplog.text -def test_2fails_then_succeed(a_fictive_discipline, caplog) -> None: +def test_2fails_then_succeed(caplog) -> None: """Test a discipline crashing the 2 first times, then succeeding.""" disc = RetryDiscipline( - a_fictive_discipline, + a_fictive_discipline(), n_trials=3, ) disc.execute() -- GitLab From 0cb11d1c7b0f1fbb69ba162290d9af0ceddc61b6 Mon Sep 17 00:00:00 2001 From: "jean-francois.figue" Date: Fri, 14 Feb 2025 17:55:29 +0100 Subject: [PATCH 3/6] test: add 2 new tests, cf #1457 --- .../wrappers/test_retry_discipline.py | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/tests/disciplines/wrappers/test_retry_discipline.py b/tests/disciplines/wrappers/test_retry_discipline.py index 7af07c2e9d..d6050a2251 100644 --- a/tests/disciplines/wrappers/test_retry_discipline.py +++ b/tests/disciplines/wrappers/test_retry_discipline.py @@ -26,6 +26,7 @@ from numpy import array from gemseo import create_discipline from gemseo.core.discipline import Discipline +from gemseo.core.execution_status import ExecutionStatus from gemseo.disciplines.wrappers.retry_discipline import RetryDiscipline from gemseo.utils.timer import Timer @@ -57,6 +58,11 @@ def a_long_time_running_discipline() -> Discipline: return DisciplineLongTimeRunning() +@pytest.fixture +def a_fictive_discipline() -> Discipline: + return FictiveDiscipline() + + class CrashingDisciplineInRun(Discipline): """A discipline raising NotImplementedError in ``_run``.""" @@ -72,6 +78,25 @@ class DisciplineLongTimeRunning(Discipline): time.sleep(5.0) +class FictiveDiscipline(Discipline): + """Discipline to be executed several times. + + - The first 2 times, raise a RuntimeError, + - and finally succeed. + """ + + def __init__(self) -> None: + super().__init__() + self.attempt = 0 + + def _run(self, input_data: StrKeyMapping) -> StrKeyMapping: + self.attempt += 1 + if self.attempt < 3: + msg = "runtime error in FictiveDiscipline" + raise RuntimeError(msg) + return {} + + @pytest.mark.parametrize("timeout", [math.inf, 10.0]) def test_retry_discipline(an_analytic_discipline, timeout, caplog) -> None: """Test discipline, no timeout set.""" @@ -232,3 +257,38 @@ def test_retry_discipline_timeout_feature( "Failed to execute discipline DisciplineLongTimeRunning after 1 attempt." ) assert log_message in caplog.text + + +@pytest.mark.parametrize("n_trials", [1, 3]) +def test_1_3times_failing(a_crashing_analytic_discipline, n_trials, caplog) -> None: + """Test a discipline crashing each time, n_trials = 1 or 3.""" + disc = RetryDiscipline( + a_crashing_analytic_discipline, + n_trials=n_trials, + ) + with pytest.raises(ZeroDivisionError, match="float division by zero"): + disc.execute({"x": array([0.0])}) + + assert disc.n_executions == n_trials + assert disc.io.data == {"x": array([0.0])} + + assert disc.execution_status.value == ExecutionStatus.Status.FAILED + + plural_suffix = "s" if n_trials > 1 else "" + log_message = ( + "Failed to execute discipline AnalyticDiscipline" + f" after {n_trials} attempt{plural_suffix}." + ) + assert log_message in caplog.text + + +def test_2fails_then_succeed(a_fictive_discipline, caplog) -> None: + """Test a discipline crashing the 2 first times, then succeeding.""" + disc = RetryDiscipline( + a_fictive_discipline, + n_trials=3, + ) + disc.execute() + assert disc.n_executions == 3 + assert disc.io.data == {} + assert disc.execution_status.value == ExecutionStatus.Status.DONE -- GitLab From 99f056b6ae3247041ea0ff0199fd72d259a50904 Mon Sep 17 00:00:00 2001 From: "jean-francois.figue" Date: Thu, 20 Feb 2025 11:12:18 +0100 Subject: [PATCH 4/6] refactor: rebase on develop --- src/gemseo/disciplines/wrappers/retry_discipline.py | 2 +- tests/disciplines/wrappers/test_retry_discipline.py | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/gemseo/disciplines/wrappers/retry_discipline.py b/src/gemseo/disciplines/wrappers/retry_discipline.py index cdcd21a828..38f999b40e 100644 --- a/src/gemseo/disciplines/wrappers/retry_discipline.py +++ b/src/gemseo/disciplines/wrappers/retry_discipline.py @@ -157,7 +157,7 @@ class RetryDiscipline(Discipline): raise current_error = error - self._discipline.execution_status.value = ExecutionStatus.Status.DONE + self.__discipline.execution_status.value = ExecutionStatus.Status.DONE time.sleep(self.wait_time) self._run_before_next_trial() diff --git a/tests/disciplines/wrappers/test_retry_discipline.py b/tests/disciplines/wrappers/test_retry_discipline.py index d6050a2251..0b6c3df36d 100644 --- a/tests/disciplines/wrappers/test_retry_discipline.py +++ b/tests/disciplines/wrappers/test_retry_discipline.py @@ -58,7 +58,6 @@ def a_long_time_running_discipline() -> Discipline: return DisciplineLongTimeRunning() -@pytest.fixture def a_fictive_discipline() -> Discipline: return FictiveDiscipline() @@ -282,10 +281,10 @@ def test_1_3times_failing(a_crashing_analytic_discipline, n_trials, caplog) -> N assert log_message in caplog.text -def test_2fails_then_succeed(a_fictive_discipline, caplog) -> None: +def test_2fails_then_succeed(caplog) -> None: """Test a discipline crashing the 2 first times, then succeeding.""" disc = RetryDiscipline( - a_fictive_discipline, + a_fictive_discipline(), n_trials=3, ) disc.execute() -- GitLab From 7ca41ea12478769b1c92a4a45d1171a123474e71 Mon Sep 17 00:00:00 2001 From: "jean-francois.figue" Date: Fri, 21 Feb 2025 09:53:35 +0100 Subject: [PATCH 5/6] fix: _discipline instead of __discipline --- src/gemseo/disciplines/wrappers/retry_discipline.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gemseo/disciplines/wrappers/retry_discipline.py b/src/gemseo/disciplines/wrappers/retry_discipline.py index 38f999b40e..cdcd21a828 100644 --- a/src/gemseo/disciplines/wrappers/retry_discipline.py +++ b/src/gemseo/disciplines/wrappers/retry_discipline.py @@ -157,7 +157,7 @@ class RetryDiscipline(Discipline): raise current_error = error - self.__discipline.execution_status.value = ExecutionStatus.Status.DONE + self._discipline.execution_status.value = ExecutionStatus.Status.DONE time.sleep(self.wait_time) self._run_before_next_trial() -- GitLab From 419ebacbf8d71f34a846d54e401de9245fea0e19 Mon Sep 17 00:00:00 2001 From: "jean-francois.figue" Date: Fri, 21 Feb 2025 10:37:43 +0100 Subject: [PATCH 6/6] test: removed useless function a_fictive_discipline --- tests/disciplines/wrappers/test_retry_discipline.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/tests/disciplines/wrappers/test_retry_discipline.py b/tests/disciplines/wrappers/test_retry_discipline.py index 0b6c3df36d..918562af58 100644 --- a/tests/disciplines/wrappers/test_retry_discipline.py +++ b/tests/disciplines/wrappers/test_retry_discipline.py @@ -58,10 +58,6 @@ def a_long_time_running_discipline() -> Discipline: return DisciplineLongTimeRunning() -def a_fictive_discipline() -> Discipline: - return FictiveDiscipline() - - class CrashingDisciplineInRun(Discipline): """A discipline raising NotImplementedError in ``_run``.""" @@ -284,7 +280,7 @@ def test_1_3times_failing(a_crashing_analytic_discipline, n_trials, caplog) -> N def test_2fails_then_succeed(caplog) -> None: """Test a discipline crashing the 2 first times, then succeeding.""" disc = RetryDiscipline( - a_fictive_discipline(), + FictiveDiscipline(), n_trials=3, ) disc.execute() -- GitLab