From bc35e30e71e344b797a9a5d4dadad54d239816bc Mon Sep 17 00:00:00 2001 From: Daniel Thwaites Date: Fri, 23 Oct 2026 11:08:56 +0100 Subject: [PATCH] Rename `IComponent` result types The old result was difficult to understand because some values had multiple meanings, and different things were valid depending on the context. Split into two results, one for de/activate and one for termination. The interesting changes are in `icomponent.hpp`. --- .../details/component.cpp | 39 ++--- .../details/component.hpp | 11 +- .../details/component_UT.cpp | 62 ++++--- .../details/component_event.hpp | 6 +- .../details/component_event_queue_UT.cpp | 4 +- .../empty_force_stop_action.cpp | 4 +- .../empty_force_stop_action.hpp | 2 +- .../empty_force_stop_action_UT.cpp | 5 +- .../force_stop_action/iforce_stop_action.hpp | 2 +- .../mock_force_stop_action.hpp | 2 +- .../process_force_stop_action.cpp | 6 +- .../process_force_stop_action.hpp | 2 +- .../process_force_stop_action_UT.cpp | 14 +- .../process_group_manager/details/graph.cpp | 16 +- .../process_group_manager/details/graph.hpp | 2 +- .../details/graph_UT.cpp | 23 ++- .../details/icomponent.hpp | 41 +++-- .../details/mock_component.hpp | 2 +- .../details/process_info_node.cpp | 83 ++++------ .../details/process_info_node.hpp | 40 ++--- .../details/process_info_node_UT.cpp | 152 +++++++----------- .../details/process_monitor.cpp | 43 ++--- .../details/process_monitor_UT.cpp | 30 +--- .../details/run_target.hpp | 8 +- .../details/safeprocessmap_UT.cpp | 2 +- .../details/stop_action/empty_stop_action.cpp | 4 +- .../details/stop_action/empty_stop_action.hpp | 2 +- .../stop_action/empty_stop_action_UT.cpp | 4 +- .../details/stop_action/istop_action.hpp | 2 +- .../details/stop_action/mock_stop_action.hpp | 2 +- .../stop_action/process_stop_action.cpp | 6 +- .../stop_action/process_stop_action.hpp | 2 +- .../stop_action/process_stop_action_UT.cpp | 12 +- .../details/transition_UT.cpp | 2 +- 34 files changed, 257 insertions(+), 380 deletions(-) diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/component.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/component.cpp index 6c692fcddc..e10bab1cfc 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/component.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/component.cpp @@ -44,7 +44,7 @@ IComponent::RequestResult Component::activate(cpp::stop_token stop_token) if (!start_result.has_value()) { state_ = FaultState{}; - return cpp::make_unexpected(ComponentError::kErrorBeforeReady); + return RequestResult::kFailure; } const Handle& handle = start_result.value(); @@ -61,7 +61,7 @@ IComponent::RequestResult Component::activate(cpp::stop_token stop_token) lock.unlock(); if (!ready_condition_.wait(stop_activation.get_token(), handle).has_value()) { - return cpp::make_unexpected(ComponentError::kErrorBeforeReady); + return RequestResult::kFailure; } lock.lock(); @@ -69,14 +69,14 @@ IComponent::RequestResult Component::activate(cpp::stop_token stop_token) // activation fails. if (stop_activation.stop_requested()) { - return cpp::make_unexpected(ComponentError::kErrorBeforeReady); + return RequestResult::kFailure; } // The state could change while the lock was released, so we have to check. if (const StartingState* const starting = std::get_if(&state_)) { state_ = ReadyState{starting->handle_}; - return RequestState::kSuccess; + return RequestResult::kSuccess; } SCORE_LANGUAGE_FUTURECPP_UNREACHABLE_MESSAGE("If stop was not requested, then state must still be StartingState"); @@ -99,29 +99,18 @@ IComponent::RequestResult Component::deactivate(cpp::stop_token stop_token) } const Result result = stop_action_.stop(stop_token, handle); - if (!result.has_value()) + if (!result.has_value() || result.value() != RequestResult::kSuccess) { state_ = FaultState{handle}; - return cpp::make_unexpected(ComponentError::kErrorAfterReady); + return RequestResult::kFailure; } - switch (*result.value()) - { - // Wait for `tryHandleTermination` to tell us the termination is done. - case RequestState::kWaiting: - state_ = TerminatingState{handle}; - return RequestState::kWaiting; - - // Already terminated, jump straight to the final state. - case RequestState::kSuccess: - state_ = TerminatedState{}; - return RequestState::kSuccess; - } - - SCORE_LANGUAGE_FUTURECPP_UNREACHABLE_MESSAGE("Every possible RequestState should be handled"); + // `tryHandleTermination` tells us when the termination is done. + state_ = TerminatingState{handle}; + return RequestResult::kSuccess; } -IComponent::RequestResult Component::tryHandleTermination(int32_t status) +IComponent::TerminationResult Component::tryHandleTermination(int32_t status) { const std::lock_guard lock{state_mutex_}; @@ -130,7 +119,7 @@ IComponent::RequestResult Component::tryHandleTermination(int32_t status) if (std::holds_alternative(state_)) { state_ = TerminatedState{}; - return RequestState::kWaiting; + return TerminationResult::kIgnore; } StartingState* const starting = std::get_if(&state_); @@ -145,13 +134,13 @@ IComponent::RequestResult Component::tryHandleTermination(int32_t status) // Certain ready conditions can accept terminations. if (starting != nullptr && ready_condition_.tryHandleTermination(handle, status)) { - return RequestState::kWaiting; + return TerminationResult::kIgnore; } // Self-terminating components can accept terminations. if (self_terminating_ && status == 0) { - return RequestState::kWaiting; + return TerminationResult::kIgnore; } // If none of the above cases apply, then termination is unexpected. @@ -162,7 +151,7 @@ IComponent::RequestResult Component::tryHandleTermination(int32_t status) } state_ = FaultState{}; - return cpp::make_unexpected(ComponentError::kErrorAfterReady); + return TerminationResult::kFailure; } IdentifierHash Component::getIdentifier() const diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/component.hpp b/score/launch_manager/src/daemon/src/process_group_manager/details/component.hpp index 9f503fa644..98eaede06e 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/component.hpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/component.hpp @@ -46,21 +46,18 @@ class Component final : public IComponent /// @brief Start the resource and set the component to active. /// @param stop_token Token which can be used to interrupt the activation. - /// @return Whether the component activation was successful, is waiting for another - /// thread, or an error was encountered. + /// @return Whether the component activation succeeded or failed. RequestResult activate(cpp::stop_token stop_token) override; /// @brief Stop the resource and set the component to inactive. /// @param stop_token Token which can be used to interrupt the deactivation. - /// @return Whether the component deactivation was successful, is waiting for another - /// thread, or an error was encountered. + /// @return Whether the component deactivation succeeded or failed. RequestResult deactivate(cpp::stop_token stop_token) override; /// @brief Notify the component that a POSIX process has terminated. /// @param status Exit code of the process. - /// @return Whether the component activation was successful, is waiting for another - /// thread, or an error was encountered. - RequestResult tryHandleTermination(int32_t status) override; + /// @return Whether the termination is ignored or means the component failed. + TerminationResult tryHandleTermination(int32_t status) override; /// @brief Return the name of the component. /// @return The name of the component. diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/component_UT.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/component_UT.cpp index bc43a0c760..5c875efcdb 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/component_UT.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/component_UT.cpp @@ -38,12 +38,12 @@ void expect_mock_handle(const Handle handle) EXPECT_EQ(std::get(handle).pid, mock_handle.pid); } -IComponent::RequestResult terminateFromOtherThread(Component& component, const int32_t status) +IComponent::TerminationResult terminateFromOtherThread(Component& component, const int32_t status) { // Termination notifications come from another thread in reality, so // replacate it in the test to catch concurrency problems. - IComponent::RequestResult result; + IComponent::TerminationResult result; std::thread termination_thread([&component, &result, status]() { result = component.tryHandleTermination(status); }); @@ -61,7 +61,7 @@ TEST(ComponentTest, StartSucceeds) const auto result = component.activate(cpp::stop_token{}); - EXPECT_TRUE(result.has_value()); + EXPECT_EQ(result, IComponent::RequestResult::kSuccess); } TEST(ComponentTest, StartActionCalled) @@ -87,13 +87,13 @@ TEST(ComponentTest, StopActionCalled) ON_CALL(start_action, start(_)).WillByDefault(Return(Result{mock_handle})); EXPECT_CALL(stop_action, stop(_, _)) - .WillOnce(DoAll(WithArg<1>(Invoke(expect_mock_handle)), Return(IComponent::RequestState::kSuccess))); + .WillOnce(DoAll(WithArg<1>(Invoke(expect_mock_handle)), Return(IComponent::RequestResult::kSuccess))); static_cast(component.activate(cpp::stop_token{})); - component.deactivate(mock_stop_token); + static_cast(component.deactivate(mock_stop_token)); } -TEST(ComponentTest, DeactivateReturnsWaitingWhenStopActionReturnsWaiting) +TEST(ComponentTest, DeactivateReturnsSuccessWhenStopActionReturnsSuccess) { MockStartAction start_action; MockStopAction stop_action; @@ -102,16 +102,15 @@ TEST(ComponentTest, DeactivateReturnsWaitingWhenStopActionReturnsWaiting) Component component(start_action, stop_action, force_stop_action, ready_condition); ON_CALL(start_action, start(_)).WillByDefault(Return(Result{mock_handle})); - ON_CALL(stop_action, stop(_, _)).WillByDefault(Return(IComponent::RequestState::kWaiting)); + ON_CALL(stop_action, stop(_, _)).WillByDefault(Return(IComponent::RequestResult::kSuccess)); static_cast(component.activate(cpp::stop_token{})); const auto result = component.deactivate(mock_stop_token); - ASSERT_TRUE(result.has_value()); - ASSERT_EQ(result.value(), IComponent::RequestState::kWaiting); + ASSERT_EQ(result, IComponent::RequestResult::kSuccess); } -TEST(ComponentTest, DeactivateReturnsSuccessWhenStopActionReturnsSuccess) +TEST(ComponentTest, DeactivateReturnsFailureWhenStopActionFails) { MockStartAction start_action; MockStopAction stop_action; @@ -120,13 +119,12 @@ TEST(ComponentTest, DeactivateReturnsSuccessWhenStopActionReturnsSuccess) Component component(start_action, stop_action, force_stop_action, ready_condition); ON_CALL(start_action, start(_)).WillByDefault(Return(Result{mock_handle})); - ON_CALL(stop_action, stop(_, _)).WillByDefault(Return(IComponent::RequestState::kSuccess)); + ON_CALL(stop_action, stop(_, _)).WillByDefault(Return(IComponent::RequestResult::kFailure)); static_cast(component.activate(cpp::stop_token{})); const auto result = component.deactivate(mock_stop_token); - ASSERT_TRUE(result.has_value()); - ASSERT_EQ(result.value(), IComponent::RequestState::kSuccess); + ASSERT_EQ(result, IComponent::RequestResult::kFailure); } TEST(ComponentTest, ReadyConditionCalled) @@ -163,7 +161,7 @@ TEST(ComponentTest, TerminationDuringStartupInterruptsActivation) const auto result = component.activate(cpp::stop_token{}); - EXPECT_FALSE(result.has_value()); + EXPECT_EQ(result, IComponent::RequestResult::kFailure); } TEST(ComponentTest, StartSetsActive) @@ -188,7 +186,7 @@ TEST(ComponentTest, StopClearsActive) Component component(start_action, stop_action, force_stop_action, ready_condition); static_cast(component.activate(cpp::stop_token{})); - component.deactivate(cpp::stop_token{}); + static_cast(component.deactivate(cpp::stop_token{})); EXPECT_FALSE(component.active()); } @@ -231,7 +229,7 @@ TEST(ComponentTest, CannotActivateInTerminatingState) Component component(start_action, stop_action, force_stop_action, ready_condition); ON_CALL(start_action, start(_)).WillByDefault(Return(Result{mock_handle})); - ON_CALL(stop_action, stop(_, _)).WillByDefault(Return(IComponent::RequestState::kWaiting)); + ON_CALL(stop_action, stop(_, _)).WillByDefault(Return(IComponent::RequestResult::kSuccess)); static_cast(component.activate(cpp::stop_token{})); static_cast(component.deactivate(cpp::stop_token{})); @@ -249,7 +247,7 @@ TEST(ComponentTest, CannotDeactivateInTerminatingState) Component component(start_action, stop_action, force_stop_action, ready_condition); ON_CALL(start_action, start(_)).WillByDefault(Return(Result{mock_handle})); - ON_CALL(stop_action, stop(_, _)).WillByDefault(Return(IComponent::RequestState::kWaiting)); + ON_CALL(stop_action, stop(_, _)).WillByDefault(Return(IComponent::RequestResult::kSuccess)); static_cast(component.activate(cpp::stop_token{})); static_cast(component.deactivate(cpp::stop_token{})); @@ -270,7 +268,7 @@ TEST(ComponentTest, CannotActivateInFaultState) ON_CALL(start_action, start(_)).WillByDefault(Return(Result{mock_handle})); static_cast(component.activate(cpp::stop_token{})); - component.tryHandleTermination(1); + static_cast(component.tryHandleTermination(1)); EXPECT_DEATH( { static_cast(component.activate(cpp::stop_token{})); }, "Cannot activate component in a running state"); @@ -287,14 +285,14 @@ TEST(ComponentTest, CannotDeactivateInFaultState) ON_CALL(start_action, start(_)).WillByDefault(Return(Result{mock_handle})); static_cast(component.activate(cpp::stop_token{})); - component.tryHandleTermination(1); + static_cast(component.tryHandleTermination(1)); EXPECT_DEATH( { static_cast(component.deactivate(cpp::stop_token{})); }, "Cannot deactivate component in a non-running state"); } -TEST(ComponentTest, SelfTerminatingExitZeroInReadyStateIsWaiting) +TEST(ComponentTest, SelfTerminatingExitZeroInReadyStateIsIgnored) { MockStartAction start_action; MockStopAction stop_action; @@ -307,8 +305,7 @@ TEST(ComponentTest, SelfTerminatingExitZeroInReadyStateIsWaiting) static_cast(component.activate(cpp::stop_token{})); const auto result = component.tryHandleTermination(0); - ASSERT_TRUE(result.has_value()); - EXPECT_EQ(result.value(), IComponent::RequestState::kWaiting); + EXPECT_EQ(result, IComponent::TerminationResult::kIgnore); } TEST(ComponentTest, SelfTerminatingExitZeroInReadyStateIsActive) @@ -327,7 +324,7 @@ TEST(ComponentTest, SelfTerminatingExitZeroInReadyStateIsActive) EXPECT_TRUE(component.active()); } -TEST(ComponentTest, SelfTerminatingExitNonZeroInReadyStateIsError) +TEST(ComponentTest, SelfTerminatingExitNonZeroInReadyStateIsFailure) { MockStartAction start_action; MockStopAction stop_action; @@ -340,8 +337,7 @@ TEST(ComponentTest, SelfTerminatingExitNonZeroInReadyStateIsError) static_cast(component.activate(cpp::stop_token{})); const auto result = component.tryHandleTermination(1); - ASSERT_FALSE(result.has_value()); - EXPECT_EQ(result.error(), IComponent::ComponentError::kErrorAfterReady); + EXPECT_EQ(result, IComponent::TerminationResult::kFailure); } TEST(ComponentTest, SelfTerminatingExitNonZeroInReadyStateIsNotActive) @@ -360,7 +356,7 @@ TEST(ComponentTest, SelfTerminatingExitNonZeroInReadyStateIsNotActive) EXPECT_FALSE(component.active()); } -TEST(ComponentTest, SelfTerminatingExitZeroInStartingStateIsWaiting) +TEST(ComponentTest, SelfTerminatingExitZeroInStartingStateIsIgnored) { MockStartAction start_action; MockStopAction stop_action; @@ -370,7 +366,7 @@ TEST(ComponentTest, SelfTerminatingExitZeroInStartingStateIsWaiting) ON_CALL(start_action, start(_)).WillByDefault(Return(Result{mock_handle})); - IComponent::RequestResult result; + IComponent::TerminationResult result; EXPECT_CALL(ready_condition, wait(_, _)).WillOnce(Invoke([&](cpp::stop_token, const Handle) { result = terminateFromOtherThread(component, 0); return Result{}; @@ -378,8 +374,7 @@ TEST(ComponentTest, SelfTerminatingExitZeroInStartingStateIsWaiting) static_cast(component.activate(cpp::stop_token{})); - ASSERT_TRUE(result.has_value()); - EXPECT_EQ(result.value(), IComponent::RequestState::kWaiting); + EXPECT_EQ(result, IComponent::TerminationResult::kIgnore); } TEST(ComponentTest, SelfTerminatingExitZeroInStartingStateIsActive) @@ -401,7 +396,7 @@ TEST(ComponentTest, SelfTerminatingExitZeroInStartingStateIsActive) EXPECT_TRUE(component.active()); } -TEST(ComponentTest, SelfTerminatingExitNonZeroInStartingStateIsError) +TEST(ComponentTest, SelfTerminatingExitNonZeroInStartingStateIsFailure) { MockStartAction start_action; MockStopAction stop_action; @@ -411,7 +406,7 @@ TEST(ComponentTest, SelfTerminatingExitNonZeroInStartingStateIsError) ON_CALL(start_action, start(_)).WillByDefault(Return(Result{mock_handle})); - IComponent::RequestResult result; + IComponent::TerminationResult result; EXPECT_CALL(ready_condition, wait(_, _)).WillOnce(Invoke([&](cpp::stop_token, const Handle) { result = terminateFromOtherThread(component, 1); return Result{}; @@ -419,8 +414,7 @@ TEST(ComponentTest, SelfTerminatingExitNonZeroInStartingStateIsError) static_cast(component.activate(cpp::stop_token{})); - ASSERT_FALSE(result.has_value()); - EXPECT_EQ(result.error(), IComponent::ComponentError::kErrorAfterReady); + EXPECT_EQ(result, IComponent::TerminationResult::kFailure); } TEST(ComponentTest, SelfTerminatingExitNonZeroInStartingStateIsNotActive) @@ -464,7 +458,7 @@ TEST(ComponentTest, SettingBothStopTokens) const auto result = component.activate(stop_source.get_token()); - EXPECT_FALSE(result.has_value()); + EXPECT_EQ(result, IComponent::RequestResult::kFailure); } } // namespace diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/component_event.hpp b/score/launch_manager/src/daemon/src/process_group_manager/details/component_event.hpp index 6ea9cd2e29..884c691fa2 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/component_event.hpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/component_event.hpp @@ -37,8 +37,8 @@ struct [[nodiscard]] ActivationFailed { /// @brief The identifier of the node that failed. IdentifierHash node_identifier; - /// @brief Description of the error. - IComponent::ComponentError reason; + /// @brief Why the activation failed, either kFailure or kTimeout. + IComponent::RequestResult reason; }; /// @brief A node finished deactivating. @@ -53,8 +53,6 @@ struct [[nodiscard]] UnexpectedTermination { /// @brief The identifier of the node that terminated. IdentifierHash node_identifier; - /// @brief Description of the error. - IComponent::ComponentError reason; }; /// @brief A job was queued but cancelled by the time it was processed diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/component_event_queue_UT.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/component_event_queue_UT.cpp index bb8afdb6ee..4723b7f0d4 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/component_event_queue_UT.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/component_event_queue_UT.cpp @@ -54,14 +54,14 @@ TEST_F(ComponentEventQueueTest, GetNextEventReturnsPushedEventWithPayloadIntact) { RecordProperty("Description", "Verify a pushed event is returned by getNextEvent with its payload preserved."); const IdentifierHash process_identifier{"payload"}; - EXPECT_TRUE(queue_.push(ActivationFailed{process_identifier, IComponent::ComponentError::kErrorBeforeReady})); + EXPECT_TRUE(queue_.push(ActivationFailed{process_identifier, IComponent::RequestResult::kFailure})); auto event = queue_.getNextEvent(); ASSERT_TRUE(event.has_value()); ASSERT_TRUE(std::holds_alternative(*event)); const auto& failed = std::get(*event); EXPECT_EQ(failed.node_identifier, process_identifier); - EXPECT_EQ(failed.reason, IComponent::ComponentError::kErrorBeforeReady); + EXPECT_EQ(failed.reason, IComponent::RequestResult::kFailure); } TEST_F(ComponentEventQueueTest, GetNextEventReturnsSupervisionFailureWithPayloadIntact) diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/force_stop_action/empty_force_stop_action.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/force_stop_action/empty_force_stop_action.cpp index ecb4d31b82..c57d7454e2 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/force_stop_action/empty_force_stop_action.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/force_stop_action/empty_force_stop_action.cpp @@ -16,9 +16,9 @@ namespace score::mw::lifecycle::internal { -Result EmptyForceStopAction::forceStop(cpp::stop_token stop_token, const Handle&) const +Result EmptyForceStopAction::forceStop(cpp::stop_token stop_token, const Handle&) const { - return IComponent::RequestState::kSuccess; + return IComponent::RequestResult::kSuccess; } } // namespace score::mw::lifecycle::internal diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/force_stop_action/empty_force_stop_action.hpp b/score/launch_manager/src/daemon/src/process_group_manager/details/force_stop_action/empty_force_stop_action.hpp index 723c18e40c..631ff114d6 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/force_stop_action/empty_force_stop_action.hpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/force_stop_action/empty_force_stop_action.hpp @@ -27,7 +27,7 @@ class EmptyForceStopAction final : public IForceStopAction /// @param stop_token Token which can be used to interrupt the action. /// @param handle The resource to act upon. /// @return Always successful. - Result forceStop(cpp::stop_token stop_token, const Handle&) const override; + Result forceStop(cpp::stop_token stop_token, const Handle&) const override; }; } // namespace score::mw::lifecycle::internal diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/force_stop_action/empty_force_stop_action_UT.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/force_stop_action/empty_force_stop_action_UT.cpp index eb452789ab..e3d1f81b3b 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/force_stop_action/empty_force_stop_action_UT.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/force_stop_action/empty_force_stop_action_UT.cpp @@ -23,10 +23,11 @@ TEST(EmptyForceStopActionTest, ForceStopSucceeds) { EmptyForceStopAction empty_force_stop_action; - const Result result = empty_force_stop_action.forceStop(cpp::stop_token{}, EmptyHandle{}); + const Result result = + empty_force_stop_action.forceStop(cpp::stop_token{}, EmptyHandle{}); ASSERT_TRUE(result.has_value()); - EXPECT_EQ(result.value(), IComponent::RequestState::kSuccess); + EXPECT_EQ(result.value(), IComponent::RequestResult::kSuccess); } } // namespace diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/force_stop_action/iforce_stop_action.hpp b/score/launch_manager/src/daemon/src/process_group_manager/details/force_stop_action/iforce_stop_action.hpp index 06d0397eee..1f708f488f 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/force_stop_action/iforce_stop_action.hpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/force_stop_action/iforce_stop_action.hpp @@ -32,7 +32,7 @@ class IForceStopAction /// @param stop_token Token which can be used to interrupt the action. /// @param handle The resource to act upon. /// @return Whether the action was successful, or failed with an error. - virtual Result forceStop(cpp::stop_token stop_token, const Handle& handle) const = 0; + virtual Result forceStop(cpp::stop_token stop_token, const Handle& handle) const = 0; }; } // namespace score::mw::lifecycle::internal diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/force_stop_action/mock_force_stop_action.hpp b/score/launch_manager/src/daemon/src/process_group_manager/details/force_stop_action/mock_force_stop_action.hpp index 744583543e..e13fd50303 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/force_stop_action/mock_force_stop_action.hpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/force_stop_action/mock_force_stop_action.hpp @@ -24,7 +24,7 @@ class MockForceStopAction : public IForceStopAction { public: MOCK_METHOD( - Result, + Result, forceStop, (cpp::stop_token, const Handle& handle), (override, const)); diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/force_stop_action/process_force_stop_action.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/force_stop_action/process_force_stop_action.cpp index a554899e7d..ed75fcae7f 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/force_stop_action/process_force_stop_action.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/force_stop_action/process_force_stop_action.cpp @@ -22,7 +22,7 @@ ProcessForceStopAction::ProcessForceStopAction(osal::IProcess& launcher) : launc { } -Result ProcessForceStopAction::forceStop(cpp::stop_token stop_token, const Handle& handle) +Result ProcessForceStopAction::forceStop(cpp::stop_token stop_token, const Handle& handle) const { const ProcessHandle* const process = std::get_if(&handle); @@ -31,14 +31,14 @@ Result ProcessForceStopAction::forceStop(cpp::stop_tok if (process->pid == -1) { - return IComponent::RequestState::kSuccess; + return IComponent::RequestResult::kSuccess; } const osal::OsalReturnType result = launcher_.forceTermination(process->pid); if (result == osal::OsalReturnType::kSuccess) { - return IComponent::RequestState::kWaiting; + return IComponent::RequestResult::kSuccess; } return MakeUnexpected(ExecErrc::kGeneralError); diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/force_stop_action/process_force_stop_action.hpp b/score/launch_manager/src/daemon/src/process_group_manager/details/force_stop_action/process_force_stop_action.hpp index d7a19f365e..121773d687 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/force_stop_action/process_force_stop_action.hpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/force_stop_action/process_force_stop_action.hpp @@ -35,7 +35,7 @@ class ProcessForceStopAction final : public IForceStopAction /// @param stop_token Token which can be used to interrupt the action. /// @param handle The resource to act upon. /// @return Whether the action was successful, or failed with an error. - Result forceStop(cpp::stop_token stop_token, const Handle& handle) const override; + Result forceStop(cpp::stop_token stop_token, const Handle& handle) const override; private: /// @brief The process launcher used to stop the process. diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/force_stop_action/process_force_stop_action_UT.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/force_stop_action/process_force_stop_action_UT.cpp index 3de0ed5343..7f7bbcc646 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/force_stop_action/process_force_stop_action_UT.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/force_stop_action/process_force_stop_action_UT.cpp @@ -36,17 +36,18 @@ TEST(ProcessForceStopActionTest, ForceStopCallsForceTerminationWithPid) static_cast(process_force_stop_action.forceStop(cpp::stop_token{}, mock_handle)); } -TEST(ProcessForceStopActionTest, ForceStopSucceedsReturnsWaiting) +TEST(ProcessForceStopActionTest, ForceStopSucceedsReturnsSuccess) { NiceMock launcher; const ProcessForceStopAction process_force_stop_action(launcher); ON_CALL(launcher, forceTermination(_)).WillByDefault(Return(osal::OsalReturnType::kSuccess)); - const Result result = process_force_stop_action.forceStop(cpp::stop_token{}, mock_handle); + const Result result = + process_force_stop_action.forceStop(cpp::stop_token{}, mock_handle); ASSERT_TRUE(result.has_value()); - EXPECT_EQ(result.value(), IComponent::RequestState::kWaiting); + EXPECT_EQ(result.value(), IComponent::RequestResult::kSuccess); } TEST(ProcessForceStopActionTest, ForceStopWithInvalidatedHandleReturnsSuccess) @@ -56,10 +57,10 @@ TEST(ProcessForceStopActionTest, ForceStopWithInvalidatedHandleReturnsSuccess) ProcessHandle handle = mock_handle; handle.pid = -1; - const Result result = process_force_stop_action.forceStop(cpp::stop_token{}, handle); + const Result result = process_force_stop_action.forceStop(cpp::stop_token{}, handle); ASSERT_TRUE(result.has_value()); - EXPECT_EQ(result.value(), IComponent::RequestState::kSuccess); + EXPECT_EQ(result.value(), IComponent::RequestResult::kSuccess); } TEST(ProcessStopActionTest, ForceStopWithInvalidatedHandleDoesNotCallLauncher) @@ -79,7 +80,8 @@ TEST(ProcessForceStopActionTest, ForceStopFailsReturnsError) ON_CALL(launcher, forceTermination(_)).WillByDefault(Return(osal::OsalReturnType::kFail)); - const Result result = process_force_stop_action.forceStop(cpp::stop_token{}, mock_handle); + const Result result = + process_force_stop_action.forceStop(cpp::stop_token{}, mock_handle); EXPECT_FALSE(result.has_value()); EXPECT_EQ(result.error(), ExecErrc::kGeneralError); diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/graph.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/graph.cpp index 846190da08..eb4a9c10ce 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/graph.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/graph.cpp @@ -175,11 +175,13 @@ void Graph::updateRunTargetInPlace(RunTarget& run_target, ComponentTaskType task // RunTargets are updated in place, no need to queue them on the thread pool. if (task_type == ComponentTaskType::kActivate) { - run_target.activate(stop_source_.get_token()); + const IComponent::RequestResult result = run_target.activate(stop_source_.get_token()); + SCORE_LANGUAGE_FUTURECPP_ASSERT(result == IComponent::RequestResult::kSuccess); } else { - run_target.deactivate(stop_source_.get_token()); + const IComponent::RequestResult result = run_target.deactivate(stop_source_.get_token()); + SCORE_LANGUAGE_FUTURECPP_ASSERT(result == IComponent::RequestResult::kSuccess); } current_transition_->onNodeFinished(run_target.getIdentifier()); } @@ -347,11 +349,11 @@ void Graph::handleComponentEvent(const ComponentEvent& event) << (std::is_same_v ? std::string_view("activation") : std::string_view("deactivation")) << "successfully"; - nodeExecuted(data.node_identifier, {}); + nodeExecuted(data.node_identifier, IComponent::RequestResult::kSuccess); } else if constexpr (std::is_same_v) { - nodeExecuted(data.node_identifier, score::cpp::make_unexpected(data.reason)); + nodeExecuted(data.node_identifier, data.reason); } else if constexpr (std::is_same_v) { @@ -368,17 +370,17 @@ void Graph::handleComponentEvent(const ComponentEvent& event) } else if constexpr (std::is_same_v) { - nodeExecuted(data.node_identifier, {}); + nodeExecuted(data.node_identifier, IComponent::RequestResult::kSuccess); } }, event); } -void Graph::nodeExecuted(IdentifierHash node, score::cpp::expected_blank error) +void Graph::nodeExecuted(IdentifierHash node, IComponent::RequestResult result) { bool was_last_in_queue = --jobs_in_progress_ == 0; - if (!error.has_value()) + if (result != IComponent::RequestResult::kSuccess) { setState(GraphState::kAborting); } diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/graph.hpp b/score/launch_manager/src/daemon/src/process_group_manager/details/graph.hpp index 76423b2e74..9769aba735 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/graph.hpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/graph.hpp @@ -279,7 +279,7 @@ class Graph final private: /// @brief Reports that a node has finished executing, enqueuing successors or updating the graph state if a /// transition has finished. - void nodeExecuted(IdentifierHash node, score::cpp::expected_blank error); + void nodeExecuted(IdentifierHash node, IComponent::RequestResult result); /// @brief Sets the current state of the graph. /// @param new_state The new state to set for the graph. diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/graph_UT.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/graph_UT.cpp index 4415691052..a28866e61e 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/graph_UT.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/graph_UT.cpp @@ -140,8 +140,7 @@ class GraphTest : public ::testing::Test res = job.component.get().deactivate(job.stop_token); } - ASSERT_TRUE(res.has_value()); - ASSERT_EQ(res.value(), IComponent::RequestState::kSuccess); + ASSERT_EQ(res, IComponent::RequestResult::kSuccess); } void failActivationJob(const ComponentTask& job) @@ -149,7 +148,7 @@ class GraphTest : public ::testing::Test EXPECT_CALL(process_interface_, startProcess).WillOnce(Return(osal::OsalReturnType::kFail)); const auto res = job.component.get().activate(job.stop_token); - ASSERT_FALSE(res.has_value()); + ASSERT_EQ(res, IComponent::RequestResult::kFailure); } /// @brief Execute a Run Target activation that activates or deactivates a single node @@ -345,7 +344,7 @@ TEST_F(GraphInitialTransitionTest, jobFailure) const auto job = job_queue_->pop()->value(); failActivationJob(job); graph_->handleComponentEvent( - ActivationFailed{IdentifierHash{process_name(0)}, IComponent::ComponentError::kErrorBeforeReady}); + ActivationFailed{IdentifierHash{process_name(0)}, IComponent::RequestResult::kFailure}); EXPECT_EQ(graph_->getState(), GraphState::kUndefinedState); } @@ -548,8 +547,7 @@ TEST_F(GraphHandleComponentEventTest, failedFirstDuringTransition) // Fail the first job const auto first_job = job_queue_->pop(); graph_->handleComponentEvent( - ActivationFailed{ - first_job->value().component.get().getIdentifier(), IComponent::ComponentError::kErrorBeforeReady}); + ActivationFailed{first_job->value().component.get().getIdentifier(), IComponent::RequestResult::kFailure}); const auto second_job = job_queue_->pop(); @@ -567,8 +565,7 @@ TEST_F(GraphHandleComponentEventTest, failureFollowedBySuccessFails) // Fail the first job const auto first_job = job_queue_->pop(); graph_->handleComponentEvent( - ActivationFailed{ - first_job->value().component.get().getIdentifier(), IComponent::ComponentError::kErrorBeforeReady}); + ActivationFailed{first_job->value().component.get().getIdentifier(), IComponent::RequestResult::kFailure}); const auto second_job = job_queue_->pop(); executeJobSuccessfully(second_job->value()); @@ -589,8 +586,7 @@ TEST_F(GraphHandleComponentEventTest, cancelAfterFailureStaysAborting) // Fail the first job const auto first_job = job_queue_->pop(); graph_->handleComponentEvent( - ActivationFailed{ - first_job->value().component.get().getIdentifier(), IComponent::ComponentError::kErrorBeforeReady}); + ActivationFailed{first_job->value().component.get().getIdentifier(), IComponent::RequestResult::kFailure}); EXPECT_EQ(graph_->getState(), GraphState::kAborting); @@ -623,8 +619,7 @@ TEST_F(GraphHandleComponentEventTest, unexpectedTerminationDuringSuccess) }), Return(osal::OsalReturnType::kSuccess))); - graph_->handleComponentEvent( - UnexpectedTermination{component->getIdentifier(), IComponent::ComponentError::kErrorAfterReady}); + graph_->handleComponentEvent(UnexpectedTermination{component->getIdentifier()}); EXPECT_EQ(graph_->getState(), GraphState::kUndefinedState); } @@ -652,7 +647,7 @@ TEST_F(GraphHandleComponentEventTest, unexpectedTerminationDuringTransition) Return(osal::OsalReturnType::kSuccess))); // The active component then crashes - graph_->handleComponentEvent(UnexpectedTermination{component_index, IComponent::ComponentError::kErrorAfterReady}); + graph_->handleComponentEvent(UnexpectedTermination{component_index}); const auto second_job = job_queue_->pop(); executeJobSuccessfully(second_job->value()); @@ -679,7 +674,7 @@ TEST_F(GraphTransitionFailuresTest, UnusualOrderOfFailures) .WillOnce(Return(osal::OsalReturnType::kSuccess)); // Process is already gone, semaphore will time out EXPECT_CALL(process_interface_, forceTermination).WillOnce(Return(osal::OsalReturnType::kFail)); - graph_->handleComponentEvent(UnexpectedTermination{component_id, IComponent::ComponentError::kErrorAfterReady}); + graph_->handleComponentEvent(UnexpectedTermination{component_id}); graph_->handleComponentEvent(ActivationSuccessful{component_id}); EXPECT_EQ(graph_->getState(), GraphState::kUndefinedState) << "Graph should be in a final state"; diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/icomponent.hpp b/score/launch_manager/src/daemon/src/process_group_manager/details/icomponent.hpp index 80df89424e..bc586b7e80 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/icomponent.hpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/icomponent.hpp @@ -31,42 +31,37 @@ namespace score::mw::lifecycle::internal class IComponent { public: - enum class ComponentError : uint8_t + enum class [[nodiscard]] RequestResult : std::uint8_t { - /// @brief An error occurred during startup, before the component was considered ready. - kErrorBeforeReady, - /// @brief An error occurred any time after the component was considered ready. - kErrorAfterReady, - /// @brief An error occurred during startup. Specifically, a timeout was reached. - kActivationTimedOut, + /// @brief The operation succeeded. + kSuccess, + + /// @brief The operation timed out. + kTimeout, + + /// @brief The operation failed. + kFailure }; - enum class RequestState : uint8_t + enum class [[nodiscard]] TerminationResult : std::uint8_t { - /// @brief Activation was successful and the component is now ready. - kSuccess, - /// @brief Activation is waiting on a notification from another thread. The component may not be ready. - kWaiting - }; + /// @brief The termination does not affect the state of the component. + kIgnore, - using RequestResult = score::cpp::expected; + /// @brief The component failed. + kFailure + }; /// @brief Begin activation of the component. /// @p stop_token Token that can be stopped to exit the activation early. - /// @returns kSuccess if the component is now ready, kWaiting if the component is waiting for a notification, or an - /// error. - [[nodiscard]] virtual RequestResult activate(score::cpp::stop_token stop_token) = 0; + virtual RequestResult activate(score::cpp::stop_token stop_token) = 0; /// @brief Begin deactivation of the component. /// @p stop_token Token that can be stopped to exit the deactivation early. - /// @returns kSuccess if the component is now ready, kWaiting if the component is waiting for a notification, or an - /// error. - [[nodiscard]] virtual RequestResult deactivate(score::cpp::stop_token stop_token) = 0; + virtual RequestResult deactivate(score::cpp::stop_token stop_token) = 0; /// @brief Notify the component that it has terminated with status @p status - /// @returns kSuccess if the component is now ready, kWaiting if the component is waiting for a notification, or an - /// error if the termination was not expected. - [[nodiscard]] virtual RequestResult tryHandleTermination(int32_t status) = 0; + virtual TerminationResult tryHandleTermination(int32_t status) = 0; /// @returns the index of the component in the graph. [[nodiscard]] virtual IdentifierHash getIdentifier() const = 0; diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/mock_component.hpp b/score/launch_manager/src/daemon/src/process_group_manager/details/mock_component.hpp index 3c04138844..8af474a3fb 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/mock_component.hpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/mock_component.hpp @@ -24,7 +24,7 @@ class MockComponent : public IComponent public: MOCK_METHOD(RequestResult, activate, (score::cpp::stop_token stop_token), (override)); MOCK_METHOD(RequestResult, deactivate, (score::cpp::stop_token stop_token), (override)); - MOCK_METHOD(RequestResult, tryHandleTermination, (int32_t status), (override)); + MOCK_METHOD(TerminationResult, tryHandleTermination, (int32_t status), (override)); MOCK_METHOD(IdentifierHash, getIdentifier, (), (override, const)); MOCK_METHOD(bool, active, (), (override, const)); }; diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.cpp index 138f68892f..c4943c16e4 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.cpp @@ -97,13 +97,13 @@ IComponent::RequestResult ProcessInfoNode::tryReportCompletion(score::mw::lifecy if (new_state == ProcessState::kFailed) { // Didn't reach running or startup - return tryReportError(ComponentError::kErrorBeforeReady); + return tryReportError(RequestResult::kFailure); } if (new_state == desired_state) { return tryReportSuccess(); } - return {IComponent::RequestState::kWaiting}; + return RequestResult::kFailure; } IComponent::RequestResult ProcessInfoNode::tryReportSuccess() @@ -112,9 +112,9 @@ IComponent::RequestResult ProcessInfoNode::tryReportSuccess() { reached_ready_.store(true); - return {RequestState::kSuccess}; + return RequestResult::kSuccess; } - return {IComponent::RequestState::kWaiting}; + return RequestResult::kFailure; } std::optional ProcessInfoNode::getTimeForAliveState() const @@ -129,14 +129,14 @@ std::optional ProcessInfoNode::getTimeForAliveState() const return std::nullopt; } -IComponent::RequestResult ProcessInfoNode::tryReportError(ComponentError error) +IComponent::RequestResult ProcessInfoNode::tryReportError(RequestResult error) { if (!success_returned_.test_and_set()) { // Activation failed to reach its ready condition. - return score::cpp::make_unexpected(error); + return error; } - return {IComponent::RequestState::kWaiting}; + return RequestResult::kFailure; } bool ProcessInfoNode::setState(score::mw::lifecycle::ProcessState new_state) @@ -173,20 +173,20 @@ void ProcessInfoNode::unblockSync() } } -IComponent::RequestResult ProcessInfoNode::tryHandleTermination(int32_t process_status) +IComponent::TerminationResult ProcessInfoNode::tryHandleTermination(int32_t process_status) { LM_LOG_DEBUG() << "Process" << identifier_ << "( pid" << pid_ << ") terminated with status" << process_status; exit_status_ = process_status; - IComponent::RequestResult res = {IComponent::RequestState::kWaiting}; + TerminationResult res = TerminationResult::kIgnore; ProcessState starting = ProcessState::kStarting; if (config_.component_properties.application_profile.is_self_terminating && process_status == 0) { - termination_result_ = TeminationResult::kOk; + termination_result_ = TerminationResult::kIgnore; } else { - termination_result_ = TeminationResult::kError; + termination_result_ = TerminationResult::kFailure; } if (termination_requested_.exchange(false)) // Termination was requested @@ -206,15 +206,18 @@ IComponent::RequestResult ProcessInfoNode::tryHandleTermination(int32_t process_ { setState(ProcessState::kTerminated); - if (termination_result_ == TeminationResult::kOk) + if (termination_result_ == TerminationResult::kIgnore) { - res = tryReportCompletion(ProcessState::kTerminated); + if (tryReportCompletion(ProcessState::kTerminated) == RequestResult::kSuccess) + { + res = TerminationResult::kIgnore; + } } else { LM_LOG_WARN() << "unexpected termination of process" << identifier_ << "( pid" << pid_ << "status" << exit_status_ << ")"; - res = score::cpp::make_unexpected(IComponent::ComponentError::kErrorAfterReady); + res = TerminationResult::kFailure; } } @@ -240,7 +243,7 @@ IComponent::RequestResult ProcessInfoNode::startProcess(score::cpp::stop_token s LM_LOG_DEBUG() << "Starting process (" << identifier_ << ") from executable" << config_.deployment_config.executable_path; - std::optional error; + std::optional error; const std::chrono::time_point initial_time = std::chrono::steady_clock::now(); for (std::uint16_t attempts = 0U; attempts <= start_retries_; attempts++) @@ -255,14 +258,14 @@ IComponent::RequestResult ProcessInfoNode::startProcess(score::cpp::stop_token s if (!setState(score::mw::lifecycle::ProcessState::kIdle)) { LM_LOG_WARN() << "Starting process" << this << "failed: termination in progress"; - error = ComponentError::kErrorBeforeReady; + error = RequestResult::kFailure; break; } pid_ = 0; exit_status_ = 0; error = std::nullopt; - termination_result_ = TeminationResult::kNone; + termination_result_ = std::nullopt; termination_waiter_->reset(); static_cast(setState(score::mw::lifecycle::ProcessState::kStarting)); // Cannot fail by design @@ -293,7 +296,7 @@ IComponent::RequestResult ProcessInfoNode::startProcess(score::cpp::stop_token s else { setState(score::mw::lifecycle::ProcessState::kFailed); - error = ComponentError::kErrorBeforeReady; + error = RequestResult::kFailure; break; } @@ -315,38 +318,20 @@ IComponent::RequestResult ProcessInfoNode::startProcess(score::cpp::stop_token s // We have terminated after starting up SCORE_LANGUAGE_FUTURECPP_ASSERT_DBG_MESSAGE( - termination_result_ != TerminationResult::kNone, "setState(kRunning) failed without a termination result"); + termination_result_.has_value(), "setState(kRunning) failed without a termination result"); // Assuming we have already waited for all ready conditions that require waiting, we have now started up and // terminated. Therefore, as long as the termination was valid, we have satisfied any running or terminated // condition. - if (termination_result_ == TeminationResult::kOk) + if (termination_result_ == TerminationResult::kIgnore) { return tryReportSuccess(); } - // We successfully reached kRunning, but reached kTerminated in error. This affects whether the error - // occurred before or after reaching the ready state - return tryReportError(getErrorAfterState(ProcessState::kRunning)); -} - -IComponent::ComponentError ProcessInfoNode::getErrorAfterState(ProcessState state_reached) const -{ - if (const configuration::ProcessState* state_condition = - std::get_if(&config_.component_properties.ready_condition)) - { - if (*state_condition == configuration::ProcessState::Terminated && state_reached == ProcessState::kRunning) - { - return IComponent::ComponentError::kErrorBeforeReady; - } - } - if (state_reached < ProcessState::kRunning) - { - return IComponent::ComponentError::kErrorBeforeReady; - } - return IComponent::ComponentError::kErrorAfterReady; + // We successfully reached kRunning, but reached kTerminated in error. + return tryReportError(RequestResult::kFailure); } -score::cpp::expected_blank ProcessInfoNode::handleProcessStillStarting( +score::cpp::expected_blank ProcessInfoNode::handleProcessStillStarting( const score::cpp::stop_token& stop_token) { const bool startup_condition_met = std::visit( @@ -434,26 +419,26 @@ score::cpp::expected_blank ProcessInfoNode::handlePr if (getState() == ProcessState::kTerminated) { - return score::cpp::make_unexpected(ComponentError::kErrorBeforeReady); + return score::cpp::make_unexpected(RequestResult::kFailure); } LM_LOG_WARN() << "Process (" << identifier_ << ") did not meet its ready condition within the ready timeout"; terminateProcess(stop_token); - return score::cpp::make_unexpected(ComponentError::kActivationTimedOut); + return score::cpp::make_unexpected(RequestResult::kTimeout); } -score::cpp::expected_blank ProcessInfoNode::handleProcessAlreadyTerminated() +score::cpp::expected_blank ProcessInfoNode::handleProcessAlreadyTerminated() { // The process did start successfully, but didn't report properly. if (isReporting()) { - return score::cpp::make_unexpected(IComponent::ComponentError::kErrorBeforeReady); + return score::cpp::make_unexpected(IComponent::RequestResult::kFailure); } // In all other cases, the process did successfully reach the running state (because pid_ was set). return {}; } -score::cpp::expected, IComponent::ComponentError> +score::cpp::expected, IComponent::RequestResult> ProcessInfoNode::handleProcessStarted(const score::cpp::stop_token& stop_token) { switch (process_handling_.process_map_->insertIfNotTerminated(pid_, this)) @@ -470,7 +455,7 @@ ProcessInfoNode::handleProcessStarted(const score::cpp::stop_token& stop_token) // really bad fatal error, should not happen, treat as a failure to set the state & kill the process LM_LOG_ERROR() << "Could not add PID to map!"; terminateProcess(stop_token); - return score::cpp::make_unexpected(ComponentError::kErrorBeforeReady); + return score::cpp::make_unexpected(RequestResult::kFailure); } } @@ -541,7 +526,7 @@ IComponent::RequestResult ProcessInfoNode::activate(score::cpp::stop_token stop_ return tryReportSuccess(); } auto res = startProcess(std::move(stop_token)); - if (res.has_value()) + if (res == RequestResult::kSuccess) { if (auto time = getTimeForAliveState()) { @@ -561,7 +546,7 @@ IComponent::RequestResult ProcessInfoNode::deactivate(score::cpp::stop_token sto } terminateProcess(stop_token); setState(ProcessState::kIdle); - return IComponent::RequestState::kSuccess; + return RequestResult::kSuccess; } bool ProcessInfoNode::active() const diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.hpp b/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.hpp index 298fc942c5..25403ac137 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.hpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.hpp @@ -25,6 +25,7 @@ #include #include #include +#include namespace score::mw::lifecycle::internal { @@ -40,17 +41,6 @@ namespace score::mw::lifecycle::internal /// In the future, this class shall be split up to properly separate Component and Process lifecycle. class ProcessInfoNode final : public IComponent { - /// @brief Enum representing different outcomes of a termination. - enum class TeminationResult : uint8_t - { - /// @brief Since the last startup, the process has not terminated. - kNone, - /// @brief The last termination was acceptable. - kOk, - /// @brief The last termination was invalid/unexpected. - kError, - }; - public: /// @brief Constructs a ProcessInfoNode. /// @param config Configuration for the OS process. @@ -92,7 +82,7 @@ class ProcessInfoNode final : public IComponent RequestResult deactivate(score::cpp::stop_token stop_token) override; - RequestResult tryHandleTermination(int32_t process_status) override; + TerminationResult tryHandleTermination(int32_t process_status) override; [[nodiscard]] bool active() const override; @@ -106,10 +96,6 @@ class ProcessInfoNode final : public IComponent std::chrono::milliseconds getTerminationTimeout() const; private: - /// @brief Given that an error has occurred after the process has reached state @p state_reached, return an error - /// indicating whether this was an error before the ready condition was satisfied, or after. - ComponentError getErrorAfterState(ProcessState state_reached) const; - /// @brief Returns true if the process is configured to report kRunning bool isReporting() const; @@ -130,14 +116,14 @@ class ProcessInfoNode final : public IComponent /// @brief Get the request result corresponding to the new state reached. For example, if the ready state is /// terminated, the function will only return kSuccess if the new state is kTerminated. - /// @return Success if the ready condition is satisfied and completion is not already reported, an error if the - /// state is unrecoverable, waiting otherwise. + /// @return kSuccess if the ready condition is satisfied and completion is not already reported, kFailure + /// otherwise. RequestResult tryReportCompletion(score::mw::lifecycle::ProcessState new_state); - /// @return The provided error if the result has not been reported yet. A waiting result otherwise. - RequestResult tryReportError(ComponentError error); + /// @return @p error (kFailure or kTimeout) if the result has not been reported yet, kFailure otherwise. + RequestResult tryReportError(RequestResult error); - /// @return Success state if success has not been returned yet, waiting result otherwise. + /// @return kSuccess if success has not been returned yet, kFailure otherwise. RequestResult tryReportSuccess(); /// @brief Requests the OS to terminate this process and waits for it to exit. @@ -145,24 +131,24 @@ class ProcessInfoNode final : public IComponent void terminateProcess(const score::cpp::stop_token& stop_token); /// @brief Starts the OS process, retrying up to the configured restart count on failure. - /// @return A success if the process reached its ready conditon, an error if startup failed, waiting otherwise. + /// @return kSuccess if the process reached its ready conditon, kFailure or kTimeout if startup failed. RequestResult startProcess(score::cpp::stop_token stop_token); /// @brief Handles the result of inserting the process into the process map after the OS /// process has been created. Routes to the appropriate handler based on whether the /// process is still running or has already terminated. /// @returns An error if the process map error is unrecoverable, the result from the relevant handler otherwise. - score::cpp::expected, ComponentError> handleProcessStarted( + score::cpp::expected, RequestResult> handleProcessStarted( const score::cpp::stop_token& stop_token); /// @brief Waits for the process to report kRunning, terminating the process if it times out. /// @post Process state is either running or terminated. /// @returns an error if the startup times out. - score::cpp::expected_blank handleProcessStillStarting(const score::cpp::stop_token& stop_token); + score::cpp::expected_blank handleProcessStillStarting(const score::cpp::stop_token& stop_token); /// @brief Handles the case where the process exited before the map insertion completed. /// @returns success only for self-terminating, non-reporting processes with exit status 0. - score::cpp::expected_blank handleProcessAlreadyTerminated(); + score::cpp::expected_blank handleProcessAlreadyTerminated(); /// @brief Logs that the process has reached the running state. void handleProcessRunning(); @@ -211,8 +197,8 @@ class ProcessInfoNode final : public IComponent /// @brief Unique hash to identify this node. IdentifierHash identifier_; - /// @brief The result of the last termination since the process started. - TeminationResult termination_result_{}; + /// @brief The result of the last termination since the process started, empty if it has not terminated. + std::optional termination_result_{}; /// @brief Waits for a process to terminate for kTerminated ready condition std::unique_ptr termination_waiter_; diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node_UT.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node_UT.cpp index d51f42abe2..ef0d4d9e7f 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node_UT.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node_UT.cpp @@ -220,8 +220,7 @@ TEST_F(ProcessInfoNodeStartupTest, CanStartNonReportingProcess) auto result = node->activate(score::cpp::stop_token{}); - ASSERT_THAT(result.has_value(), IsTrue()); - ASSERT_THAT(result.value(), Eq(IComponent::RequestState::kSuccess)); + ASSERT_THAT(result, Eq(IComponent::RequestResult::kSuccess)); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kRunning)); } @@ -236,32 +235,30 @@ TEST_F(ProcessInfoNodeStartupTest, CanStartReportingProcess_ReportsRunningInTime auto result = node->activate(score::cpp::stop_token{}); - ASSERT_THAT(result.has_value(), IsTrue()); - ASSERT_THAT(result.value(), Eq(IComponent::RequestState::kSuccess)); + ASSERT_THAT(result, Eq(IComponent::RequestResult::kSuccess)); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kRunning)); } -TEST_F(ProcessInfoNodeStartupTest, OsForkFails_ReturnsErrorBeforeReady) +TEST_F(ProcessInfoNodeStartupTest, OsForkFails_ReturnsFailure) { RecordProperty( "Description", - "If the OS fails to fork the process, activate() returns kErrorBeforeReady and the node ends up in kFailed."); + "If the OS fails to fork the process, activate() returns kFailure and the node ends up in kFailed."); auto node = createProcessInfoNode(configuration::ApplicationType::Native); EXPECT_CALL(mock_processIf_, startProcess(_, _, _)).WillOnce(Return(osal::OsalReturnType::kFail)); auto result = node->activate(score::cpp::stop_token{}); - ASSERT_THAT(result.has_value(), IsFalse()); - ASSERT_THAT(result.error(), Eq(IComponent::ComponentError::kErrorBeforeReady)); + ASSERT_THAT(result, Eq(IComponent::RequestResult::kFailure)); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kFailed)); } -TEST_F(ProcessInfoNodeStartupTest, MapInsertError_ReturnsErrorBeforeReady) +TEST_F(ProcessInfoNodeStartupTest, MapInsertError_ReturnsFailure) { RecordProperty( "Description", - "If the process map insertion fails with an error, activate() returns kErrorBeforeReady and the " + "If the process map insertion fails with an error, activate() returns kFailure and the " "node ends up in kFailed."); auto node = createProcessInfoNode(configuration::ApplicationType::Native); @@ -273,8 +270,7 @@ TEST_F(ProcessInfoNodeStartupTest, MapInsertError_ReturnsErrorBeforeReady) auto result = node->activate(score::cpp::stop_token{}); - ASSERT_THAT(result.has_value(), IsFalse()); - ASSERT_THAT(result.error(), Eq(IComponent::ComponentError::kErrorBeforeReady)); + ASSERT_THAT(result, Eq(IComponent::RequestResult::kFailure)); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kFailed)); } @@ -298,8 +294,7 @@ TEST_F(ProcessInfoNodeStartupTest, SelfTerminating_ExitsBeforeMapInsert_ReturnsS auto result = node->activate(score::cpp::stop_token{}); - ASSERT_THAT(result.has_value(), IsTrue()); - ASSERT_THAT(result.value(), Eq(IComponent::RequestState::kSuccess)); + ASSERT_THAT(result, Eq(IComponent::RequestResult::kSuccess)); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kTerminated)); } @@ -313,13 +308,12 @@ TEST_F(ProcessInfoNodeStartupTest, ActivateAlreadyActiveNode_ReturnsSuccess) auto result = node->activate(score::cpp::stop_token{}); - ASSERT_THAT(result.has_value(), IsTrue()); - ASSERT_THAT(result.value(), Eq(IComponent::RequestState::kSuccess)); + ASSERT_THAT(result, Eq(IComponent::RequestResult::kSuccess)); ASSERT_THAT(node->active(), IsTrue()); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kRunning)); } -TEST_F(ProcessInfoNodeStartupTest, TerminatedReadyCondition_StopRequestedDuringStartup_ReturnsActivationTimedOut) +TEST_F(ProcessInfoNodeStartupTest, TerminatedReadyCondition_StopRequestedDuringStartup_ReturnsTimeout) { RecordProperty( "Description", @@ -338,12 +332,11 @@ TEST_F(ProcessInfoNodeStartupTest, TerminatedReadyCondition_StopRequestedDuringS auto result = node->activate(stop_source_.get_token()); - ASSERT_THAT(result.has_value(), IsFalse()); - ASSERT_THAT(result.error(), Eq(IComponent::ComponentError::kActivationTimedOut)); + ASSERT_THAT(result, Eq(IComponent::RequestResult::kTimeout)); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kTerminated)); } -TEST_F(ProcessInfoNodeStartupTest, TerminatedReadyCondition_NoTerminationInTime_ReturnsActivationTimedOut) +TEST_F(ProcessInfoNodeStartupTest, TerminatedReadyCondition_NoTerminationInTime_ReturnsTimeout) { RecordProperty( "Description", @@ -359,8 +352,7 @@ TEST_F(ProcessInfoNodeStartupTest, TerminatedReadyCondition_NoTerminationInTime_ auto result = node->activate(score::cpp::stop_token{}); - ASSERT_THAT(result.has_value(), IsFalse()); - ASSERT_THAT(result.error(), Eq(IComponent::ComponentError::kActivationTimedOut)); + ASSERT_THAT(result, Eq(IComponent::RequestResult::kTimeout)); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kTerminated)); } @@ -391,7 +383,7 @@ TEST_P(ProcessInfoNodeMapYieldTest, InsertReturnsYield) RecordProperty("Description", GetParam().description_); auto node = createProcessInfoNode(GetParam().app_type_, 0, GetParam().is_self_terminating_, GetParam().condition_); - IComponent::RequestResult tryHandleTerminationResult; + IComponent::TerminationResult tryHandleTerminationResult{}; auto status = GetParam().status_to_exit_with_; EXPECT_CALL(mock_processIf_, startProcess).WillOnce(Return(osal::OsalReturnType::kSuccess)); @@ -406,18 +398,8 @@ TEST_P(ProcessInfoNodeMapYieldTest, InsertReturnsYield) auto activation_result_ = node->activate(score::cpp::stop_token{}); - ASSERT_EQ(activation_result_.has_value(), GetParam().expected_activation_result_.has_value()); - if (GetParam().expected_activation_result_.has_value()) - { - EXPECT_EQ(activation_result_.value(), GetParam().expected_activation_result_.value()); - } - else - { - EXPECT_EQ(activation_result_.error(), GetParam().expected_activation_result_.error()); - } - - ASSERT_TRUE(tryHandleTerminationResult.has_value()); - EXPECT_EQ(tryHandleTerminationResult.value(), IComponent::RequestState::kWaiting) + EXPECT_EQ(activation_result_, GetParam().expected_activation_result_); + EXPECT_EQ(tryHandleTerminationResult, IComponent::TerminationResult::kIgnore) << "An error occurring during startup should never be reported by tryHandleTermination"; EXPECT_EQ(node->getState(), ProcessState::kTerminated); // Not kFailed, the posix process did start successfully } @@ -431,14 +413,14 @@ INSTANTIATE_TEST_SUITE_P( configuration::ProcessState::Running, 0, true, - {IComponent::RequestState::kSuccess}, + IComponent::RequestResult::kSuccess, "A native, self-terminating process exiting quickly with status 0 should report a successful activation"}, YieldTestCasesData{ configuration::ApplicationType::Native, configuration::ProcessState::Running, 111, true, - score::cpp::make_unexpected(IComponent::ComponentError::kErrorAfterReady), + IComponent::RequestResult::kFailure, "A native, self-terminating process exiting quickly with a non-zero status should report a failure to " "activate"}, YieldTestCasesData{ @@ -446,14 +428,14 @@ INSTANTIATE_TEST_SUITE_P( configuration::ProcessState::Running, 0, false, - score::cpp::make_unexpected(IComponent::ComponentError::kErrorAfterReady), + IComponent::RequestResult::kFailure, "A native, non-self-terminating process exiting quickly should report a failure to activate"}, YieldTestCasesData{ configuration::ApplicationType::Reporting, configuration::ProcessState::Running, 0, true, - score::cpp::make_unexpected(IComponent::ComponentError::kErrorBeforeReady), + IComponent::RequestResult::kFailure, "A reporting process exiting quickly (i.e. without waiting for a response from launch manager) should " "report a failure to activate"}, @@ -462,14 +444,14 @@ INSTANTIATE_TEST_SUITE_P( configuration::ProcessState::Terminated, 0, true, - {IComponent::RequestState::kSuccess}, + IComponent::RequestResult::kSuccess, "A native, self-terminating process exiting quickly with status 0 should report a successful activation"}, YieldTestCasesData{ configuration::ApplicationType::Native, configuration::ProcessState::Terminated, 111, true, - score::cpp::make_unexpected(IComponent::ComponentError::kErrorBeforeReady), + IComponent::RequestResult::kFailure, "A native, self-terminating process exiting quickly with a non-zero status should report a failure to " "activate"}, YieldTestCasesData{ @@ -477,14 +459,14 @@ INSTANTIATE_TEST_SUITE_P( configuration::ProcessState::Terminated, 0, false, - score::cpp::make_unexpected(IComponent::ComponentError::kErrorBeforeReady), + IComponent::RequestResult::kFailure, "A native, non-self-terminating process exiting quickly should report a failure to activate"}, YieldTestCasesData{ configuration::ApplicationType::Reporting, configuration::ProcessState::Terminated, 0, true, - score::cpp::make_unexpected(IComponent::ComponentError::kErrorBeforeReady), + IComponent::RequestResult::kFailure, "A reporting process exiting quickly (i.e. without waiting for a response from launch manager) should " "report a failure to activate"})); @@ -496,8 +478,7 @@ class ProcessInfoNodeStartupCrashTest : public ProcessInfoNodeFixture TEST_F(ProcessInfoNodeStartupCrashTest, ProcesssTerminated_OnWaitForkRunningTimeout) { RecordProperty( - "Description", - "If waitForkRunning times out, the process reports kActivationTimedOut and ends up in state kTerminated."); + "Description", "If waitForkRunning times out, the process reports kTimeout and ends up in state kTerminated."); expectActivationReport(0); auto node = createProcessInfoNode(configuration::ApplicationType::ReportingAndSupervised); @@ -508,8 +489,7 @@ TEST_F(ProcessInfoNodeStartupCrashTest, ProcesssTerminated_OnWaitForkRunningTime auto result = node->activate(score::cpp::stop_token{}); - ASSERT_THAT(result.has_value(), IsFalse()); - ASSERT_THAT(result.error(), Eq(IComponent::ComponentError::kActivationTimedOut)); + ASSERT_THAT(result, Eq(IComponent::RequestResult::kTimeout)); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kTerminated)); } @@ -517,7 +497,7 @@ TEST_F(ProcessInfoNodeStartupCrashTest, ReportingProcess_CrashesBeforeReady_NoRe { RecordProperty( "Description", - "Process returns kErrorBeforeReady when crashing before reaching its ready condition (kRunning) with 0 restart " + "Process returns kFailure when crashing before reaching its ready condition (kRunning) with 0 restart " "attempts"); expectActivationReport(0); @@ -533,8 +513,7 @@ TEST_F(ProcessInfoNodeStartupCrashTest, ReportingProcess_CrashesBeforeReady_NoRe auto result = node->activate(score::cpp::stop_token{}); - ASSERT_THAT(result.has_value(), IsFalse()); - ASSERT_THAT(result.error(), Eq(IComponent::ComponentError::kErrorBeforeReady)); + ASSERT_THAT(result, Eq(IComponent::RequestResult::kFailure)); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kTerminated)); } @@ -542,7 +521,7 @@ TEST_F(ProcessInfoNodeStartupCrashTest, ReportingProcess_CrashesBeforeReady_With { RecordProperty( "Description", - "Process returns kErrorBeforeReady when crashing before reaching its ready condition (kRunning) with 3 restart " + "Process returns kFailure when crashing before reaching its ready condition (kRunning) with 3 restart " "attempts"); expectActivationReport(0); @@ -568,8 +547,7 @@ TEST_F(ProcessInfoNodeStartupCrashTest, ReportingProcess_CrashesBeforeReady_With auto result = node->activate(score::cpp::stop_token{}); - ASSERT_THAT(result.has_value(), IsFalse()); - ASSERT_THAT(result.error(), Eq(IComponent::ComponentError::kErrorBeforeReady)); + ASSERT_THAT(result, Eq(IComponent::RequestResult::kFailure)); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kTerminated)); } @@ -578,7 +556,7 @@ TEST_F(ProcessInfoNodeStartupCrashTest, NonReportingProcess_CrashesBeforeReady_N RecordProperty( "Description", "A non-reporting process that crashes (non-zero status) between map insertion and the startup thread's status " - "check returns kErrorBeforeReady."); + "check returns kFailure."); auto node = createProcessInfoNode(configuration::ApplicationType::Native); EXPECT_CALL(mock_processIf_, startProcess(_, _, _)).WillOnce(Return(osal::OsalReturnType::kSuccess)); @@ -592,8 +570,7 @@ TEST_F(ProcessInfoNodeStartupCrashTest, NonReportingProcess_CrashesBeforeReady_N auto result = node->activate(score::cpp::stop_token{}); - ASSERT_THAT(result.has_value(), IsFalse()); - ASSERT_THAT(result.error(), Eq(IComponent::ComponentError::kErrorBeforeReady)); + ASSERT_THAT(result, Eq(IComponent::RequestResult::kFailure)); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kTerminated)); } @@ -602,7 +579,7 @@ TEST_F(ProcessInfoNodeStartupCrashTest, NonReportingProcess_CrashesBeforeReady_W RecordProperty( "Description", "A non-reporting process that crashes before ready on every attempt exhausts all restart attempts and returns " - "kErrorBeforeReady."); + "kFailure."); constexpr uint32_t kRestartAttempts = 2; constexpr uint32_t kTotalAttempts = kRestartAttempts + 1; @@ -621,8 +598,7 @@ TEST_F(ProcessInfoNodeStartupCrashTest, NonReportingProcess_CrashesBeforeReady_W auto result = node->activate(score::cpp::stop_token{}); - ASSERT_THAT(result.has_value(), IsFalse()); - ASSERT_THAT(result.error(), Eq(IComponent::ComponentError::kErrorBeforeReady)); + ASSERT_THAT(result, Eq(IComponent::RequestResult::kFailure)); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kTerminated)); } @@ -649,8 +625,7 @@ TEST_F(ProcessInfoNodeStartupCrashTest, TimeoutThenSuccess_WithRestarts) auto result = node->activate(score::cpp::stop_token{}); - ASSERT_THAT(result.has_value(), IsTrue()); - ASSERT_THAT(result.value(), Eq(IComponent::RequestState::kSuccess)); + ASSERT_THAT(result, Eq(IComponent::RequestResult::kSuccess)); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kRunning)); } @@ -679,7 +654,7 @@ TEST_P( "Expect that the OS handler detects a crash on every attempt. " "Expect that the number of start attempts equals the expected values: [1, 2, 255, 256]. " "When activate() is called. " - "Then kErrorBeforeReady is returned for all number_of_attempts values. "); + "Then kFailure is returned for all number_of_attempts values. "); // Expect expectActivationReport(0); @@ -707,8 +682,7 @@ TEST_P( auto result = node->activate(score::cpp::stop_token{}); // Then - ASSERT_THAT(result.has_value(), IsFalse()); - ASSERT_THAT(result.error(), Eq(IComponent::ComponentError::kErrorBeforeReady)); + ASSERT_THAT(result, Eq(IComponent::RequestResult::kFailure)); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kTerminated)); } @@ -728,14 +702,13 @@ class ProcessInfoNodeUnexpectedTerminationTest : public ProcessInfoNodeFixture TEST_F(ProcessInfoNodeUnexpectedTerminationTest, ProcesssCrashed_AfterReadyCondition) { RecordProperty( - "Description", "Process returns kErrorAfterReady when crashing after reaching its ready condition (kRunning)."); + "Description", "Process returns kFailure when crashing after reaching its ready condition (kRunning)."); auto node = createRunningProcessInfoNode(configuration::ApplicationType::Native); auto result = node->tryHandleTermination(-1); - ASSERT_THAT(result.has_value(), IsFalse()); - ASSERT_THAT(result.error(), Eq(IComponent::ComponentError::kErrorAfterReady)); + ASSERT_THAT(result, Eq(IComponent::TerminationResult::kFailure)); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kTerminated)); } @@ -751,8 +724,7 @@ TEST_F(ProcessInfoNodeUnexpectedTerminationTest, SelfTerminatingProcess_ExitsWit auto result = node->tryHandleTermination(0); - ASSERT_THAT(result.has_value(), IsTrue()); - ASSERT_THAT(result.value(), Eq(IComponent::RequestState::kWaiting)); + ASSERT_THAT(result, Eq(IComponent::TerminationResult::kIgnore)); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kTerminated)); } @@ -760,8 +732,8 @@ TEST_F(ProcessInfoNodeUnexpectedTerminationTest, SelfTerminating_TerminatedReady { RecordProperty( "Description", - "A self-terminating process with ReadyCondition::kTerminated returns kSuccess from tryHandleTermination() when " - "it exits cleanly, since its exit is the event that satisfies the ready condition."); + "A self-terminating process with ReadyCondition::kTerminated exits cleanly while activate() is waiting for " + "it, so activate() returns kSuccess and tryHandleTermination() returns kIgnore."); auto node = createProcessInfoNode( configuration::ApplicationType::Native, @@ -770,7 +742,7 @@ TEST_F(ProcessInfoNodeUnexpectedTerminationTest, SelfTerminating_TerminatedReady configuration::ProcessState::Terminated /*ready condition*/); // std::future to hold the asynchronous termination handler execution - std::future termination_future; + std::future termination_future; EXPECT_CALL(mock_processIf_, startProcess(_, _, _)).WillOnce(Return(osal::OsalReturnType::kSuccess)); @@ -785,21 +757,19 @@ TEST_F(ProcessInfoNodeUnexpectedTerminationTest, SelfTerminating_TerminatedReady // activate() returns kSuccess because kTerminated (the ready condition) is reached within the timeout auto activate_result = node->activate(score::cpp::stop_token{}); - ASSERT_THAT(activate_result.has_value(), IsTrue()); - ASSERT_THAT(activate_result.value(), Eq(IComponent::RequestState::kSuccess)); + ASSERT_THAT(activate_result, Eq(IComponent::RequestResult::kSuccess)); auto termination_result = termination_future.get(); - ASSERT_THAT(termination_result.has_value(), IsTrue()); - ASSERT_THAT(termination_result.value(), Eq(IComponent::RequestState::kWaiting)); + ASSERT_THAT(termination_result, Eq(IComponent::TerminationResult::kIgnore)); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kTerminated)); } -TEST_F(ProcessInfoNodeUnexpectedTerminationTest, SelfTerminating_CrashAfterReady_ReturnsErrorAfterReady) +TEST_F(ProcessInfoNodeUnexpectedTerminationTest, SelfTerminating_CrashAfterReady_ReturnsFailure) { RecordProperty( "Description", "A self-terminating process that crashes (non-zero exit status) after reaching its ready condition returns " - "kErrorAfterReady, just like a non-self-terminating process crash."); + "kFailure, just like a non-self-terminating process crash."); auto node = createSelfTerminatingProcessInfoNode(configuration::ApplicationType::Native); expectSuccessfulProcessLaunch(); @@ -807,8 +777,7 @@ TEST_F(ProcessInfoNodeUnexpectedTerminationTest, SelfTerminating_CrashAfterReady auto result = node->tryHandleTermination(-1); - ASSERT_THAT(result.has_value(), IsFalse()); - ASSERT_THAT(result.error(), Eq(IComponent::ComponentError::kErrorAfterReady)); + ASSERT_THAT(result, Eq(IComponent::TerminationResult::kFailure)); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kTerminated)); } @@ -833,8 +802,7 @@ TEST_F(ProcessInfoNodeDeactivationTest, CanTerminateNonSelfTerminatingProcess) auto result = node->deactivate(score::cpp::stop_token{}); - ASSERT_THAT(result.has_value(), IsTrue()); - ASSERT_THAT(result.value(), Eq(IComponent::RequestState::kSuccess)); + ASSERT_THAT(result, Eq(IComponent::RequestResult::kSuccess)); ASSERT_THAT(node->active(), IsFalse()); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kIdle)); } @@ -900,8 +868,7 @@ TEST_F(ProcessInfoNodeDeactivationTest, ProcessIgnoresSigterm_ForcedWithSigkill) auto result = node->deactivate(score::cpp::stop_token{}); - ASSERT_THAT(result.has_value(), IsTrue()); - ASSERT_THAT(result.value(), Eq(IComponent::RequestState::kSuccess)); + ASSERT_THAT(result, Eq(IComponent::RequestResult::kSuccess)); ASSERT_THAT(node->active(), IsFalse()); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kIdle)); } @@ -937,8 +904,7 @@ TEST_F(ProcessInfoNodeFileStateTest, ConditionAlreadyMet_ReturnsSuccess) auto result = node->activate(score::cpp::stop_token{}); - ASSERT_THAT(result.has_value(), IsTrue()); - ASSERT_THAT(result.value(), Eq(IComponent::RequestState::kSuccess)); + ASSERT_THAT(result, Eq(IComponent::RequestResult::kSuccess)); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kRunning)); } @@ -957,8 +923,7 @@ TEST_F(ProcessInfoNodeFileStateTest, NotExistingCondition_ReturnsSuccess) auto result = node->activate(score::cpp::stop_token{}); - ASSERT_THAT(result.has_value(), IsTrue()); - ASSERT_THAT(result.value(), Eq(IComponent::RequestState::kSuccess)); + ASSERT_THAT(result, Eq(IComponent::RequestResult::kSuccess)); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kRunning)); } @@ -973,16 +938,14 @@ TEST_F(ProcessInfoNodeFileStateTest, NativeApplication_DoesNotIgnoreRunning_Retu auto result = node->activate(score::cpp::stop_token{}); - ASSERT_THAT(result.has_value(), IsTrue()); - ASSERT_THAT(result.value(), Eq(IComponent::RequestState::kSuccess)); + ASSERT_THAT(result, Eq(IComponent::RequestResult::kSuccess)); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kRunning)); } -TEST_F(ProcessInfoNodeFileStateTest, WaitForFileTimesOut_ReturnsActivationTimedOut) +TEST_F(ProcessInfoNodeFileStateTest, WaitForFileTimesOut_ReturnsTimeout) { RecordProperty( - "Description", - "If waitForFile() times out, activate() returns kActivationTimedOut and the process ends up terminated."); + "Description", "If waitForFile() times out, activate() returns kTimeout and the process ends up terminated."); auto node = createFileStateProcessInfoNode("/var/run/ready", configuration::FileExistenceState::Exists); expectSuccessfulProcessLaunch(); @@ -993,8 +956,7 @@ TEST_F(ProcessInfoNodeFileStateTest, WaitForFileTimesOut_ReturnsActivationTimedO auto result = node->activate(score::cpp::stop_token{}); - ASSERT_THAT(result.has_value(), IsFalse()); - ASSERT_THAT(result.error(), Eq(IComponent::ComponentError::kActivationTimedOut)); + ASSERT_THAT(result, Eq(IComponent::RequestResult::kTimeout)); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kTerminated)); } diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/process_monitor.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/process_monitor.cpp index 3f8e6a79d2..a856fe3d93 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/process_monitor.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/process_monitor.cpp @@ -62,7 +62,7 @@ void ProcessMonitor::doWork(ComponentTask&& task) } }; - auto handle_failure = [&](IComponent::ComponentError& error) { + auto handle_failure = [&](IComponent::RequestResult error) { const IdentifierHash node_identifier = task.component.get().getIdentifier(); switch (task.type) @@ -78,17 +78,6 @@ void ProcessMonitor::doWork(ComponentTask&& task) } }; - auto handle_state = [&](IComponent::RequestState& state) { - switch (state) - { - case IComponent::RequestState::kSuccess: - handle_success(); - break; - case IComponent::RequestState::kWaiting: - break; - } - }; - if (task.stop_token.stop_requested()) { if (!event_queue_.push(JobSkipped{task.component.get().getIdentifier()})) @@ -98,29 +87,31 @@ void ProcessMonitor::doWork(ComponentTask&& task) return; } - auto result = run_task(); + const IComponent::RequestResult result = run_task(); - if (result.has_value()) - { - handle_state(result.value()); - } - else + switch (result) { - handle_failure(result.error()); + case IComponent::RequestResult::kSuccess: + handle_success(); + break; + case IComponent::RequestResult::kTimeout: + case IComponent::RequestResult::kFailure: + handle_failure(result); + break; } } void ProcessMonitor::terminated(IComponent& component, int32_t status) { - auto res = component.tryHandleTermination(status); bool push_res = true; - if (!res.has_value()) - { - push_res = event_queue_.push(UnexpectedTermination{component.getIdentifier(), res.error()}); - } - else if (res.value() != IComponent::RequestState::kWaiting) + switch (component.tryHandleTermination(status)) { - push_res = event_queue_.push(ActivationSuccessful{component.getIdentifier()}); + case IComponent::TerminationResult::kFailure: + push_res = event_queue_.push(UnexpectedTermination{component.getIdentifier()}); + break; + case IComponent::TerminationResult::kIgnore: + LM_LOG_DEBUG() << "Termination of component" << component.getIdentifier() << "is expected"; + break; } if (!push_res) diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/process_monitor_UT.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/process_monitor_UT.cpp index b4cde272c7..0e9988a7b8 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/process_monitor_UT.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/process_monitor_UT.cpp @@ -44,7 +44,7 @@ TEST_F(ProcessMonitorTest, DoWorkNormalActivation) RecordProperty("Description", "Verify that the process monitor activates components during activate tasks"); // Given a valid activate task // Then - EXPECT_CALL(mock_component, activate).WillOnce(Return(IComponent::RequestState::kSuccess)); + EXPECT_CALL(mock_component, activate).WillOnce(Return(IComponent::RequestResult::kSuccess)); EXPECT_CALL( mock_queue, push(VariantWith(Field(&ActivationSuccessful::node_identifier, kDefaultIdentifier)))) @@ -58,7 +58,7 @@ TEST_F(ProcessMonitorTest, DoWorkNormalDeactivation) RecordProperty("Description", "Verify that the process monitor deactivates components during deactivate tasks"); // Given a valid activate task // Then - EXPECT_CALL(mock_component, deactivate).WillOnce(Return(IComponent::RequestState::kSuccess)); + EXPECT_CALL(mock_component, deactivate).WillOnce(Return(IComponent::RequestResult::kSuccess)); EXPECT_CALL( mock_queue, push(VariantWith(Field(&DeactivationComplete::node_identifier, kDefaultIdentifier)))) @@ -67,25 +67,6 @@ TEST_F(ProcessMonitorTest, DoWorkNormalDeactivation) process_monitor.doWork(ComponentTask{ComponentTaskType::kDeactivate, mock_component, stop_source.get_token()}); } -TEST_F(ProcessMonitorTest, DoWorkOnTerminationDepProcess) -{ - RecordProperty( - "Description", "Verify that a process activated by its own termination recieves the correct instructions"); - // Given a task for starting a component that only reaches active when it terminates - // Then - // The component is neither complete nor failed - EXPECT_CALL(mock_component, activate).WillOnce(Return(IComponent::RequestState::kWaiting)); - EXPECT_CALL(mock_component, tryHandleTermination).WillOnce(Return(IComponent::RequestState::kSuccess)); - EXPECT_CALL( - mock_queue, - push(VariantWith(Field(&ActivationSuccessful::node_identifier, kDefaultIdentifier)))) - .Times(1); - // When - process_monitor.doWork(ComponentTask{ComponentTaskType::kActivate, mock_component, stop_source.get_token()}); - // The OS thread detects the termination: - process_monitor.terminated(mock_component, 0); -} - TEST_F(ProcessMonitorTest, TerminatedUnexpectedly) { RecordProperty( @@ -94,8 +75,7 @@ TEST_F(ProcessMonitorTest, TerminatedUnexpectedly) // Given a terminated signal on a component that is not expected to terminate: // Then - EXPECT_CALL(mock_component, tryHandleTermination) - .WillOnce(Return(score::cpp::make_unexpected(IComponent::ComponentError::kErrorAfterReady))); + EXPECT_CALL(mock_component, tryHandleTermination).WillOnce(Return(IComponent::TerminationResult::kFailure)); EXPECT_CALL( mock_queue, push(VariantWith(Field(&UnexpectedTermination::node_identifier, kDefaultIdentifier)))) @@ -111,9 +91,9 @@ TEST_F(ProcessMonitorTest, ActivationFailed) "Verify that when a component activation fails, the correct error and data is pushed to the event queue"); // Given a task that will fail to activate - auto errc = IComponent::ComponentError::kActivationTimedOut; + auto errc = IComponent::RequestResult::kTimeout; // Then - EXPECT_CALL(mock_component, activate).WillOnce(Return(score::cpp::make_unexpected(errc))); + EXPECT_CALL(mock_component, activate).WillOnce(Return(errc)); EXPECT_CALL(mock_queue, push(VariantWith(Field(&ActivationFailed::reason, errc)))).Times(1); // When process_monitor.doWork(ComponentTask{ComponentTaskType::kActivate, mock_component, stop_source.get_token()}); diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/run_target.hpp b/score/launch_manager/src/daemon/src/process_group_manager/details/run_target.hpp index 1cfd51b832..2653921c8e 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/run_target.hpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/run_target.hpp @@ -42,18 +42,18 @@ class RunTarget final : public IComponent RequestResult activate(score::cpp::stop_token) override { active_ = true; - return RequestState::kSuccess; + return RequestResult::kSuccess; } RequestResult deactivate(score::cpp::stop_token) override { active_ = false; - return RequestState::kSuccess; + return RequestResult::kSuccess; } - RequestResult tryHandleTermination(int32_t) override + TerminationResult tryHandleTermination(int32_t) override { - return RequestState::kSuccess; + return TerminationResult::kIgnore; } IdentifierHash getIdentifier() const override diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/safeprocessmap_UT.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/safeprocessmap_UT.cpp index cace64d1e7..8da95aded4 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/safeprocessmap_UT.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/safeprocessmap_UT.cpp @@ -47,7 +47,7 @@ class MockComponent : public IComponent public: MOCK_METHOD(RequestResult, activate, (score::cpp::stop_token stop_token), (override)); MOCK_METHOD(RequestResult, deactivate, (score::cpp::stop_token stop_token), (override)); - MOCK_METHOD(RequestResult, tryHandleTermination, (int32_t status), (override)); + MOCK_METHOD(TerminationResult, tryHandleTermination, (int32_t status), (override)); MOCK_METHOD(bool, active, (), (const override)); MOCK_METHOD(bool, stopped, (), (const override)); MOCK_METHOD(score::mw::lifecycle::IdentifierHash, getIdentifier, (), (const override)); diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/stop_action/empty_stop_action.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/stop_action/empty_stop_action.cpp index 14cd03a53d..f57051fe1d 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/stop_action/empty_stop_action.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/stop_action/empty_stop_action.cpp @@ -16,9 +16,9 @@ namespace score::mw::lifecycle::internal { -Result EmptyStopAction::stop(cpp::stop_token stop_token, const Handle&) const +Result EmptyStopAction::stop(cpp::stop_token stop_token, const Handle&) const { - return IComponent::RequestState::kSuccess; + return IComponent::RequestResult::kSuccess; } } // namespace score::mw::lifecycle::internal diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/stop_action/empty_stop_action.hpp b/score/launch_manager/src/daemon/src/process_group_manager/details/stop_action/empty_stop_action.hpp index 38f3f1309d..aa46b15742 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/stop_action/empty_stop_action.hpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/stop_action/empty_stop_action.hpp @@ -27,7 +27,7 @@ class EmptyStopAction final : public IStopAction /// @param stop_token Token which can be used to interrupt the action. /// @param handle The resource to act upon. /// @return Always successful. - Result stop(cpp::stop_token stop_token, const Handle&) const override; + Result stop(cpp::stop_token stop_token, const Handle&) const override; }; } // namespace score::mw::lifecycle::internal diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/stop_action/empty_stop_action_UT.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/stop_action/empty_stop_action_UT.cpp index 120007ff57..5451ef3ff0 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/stop_action/empty_stop_action_UT.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/stop_action/empty_stop_action_UT.cpp @@ -23,10 +23,10 @@ TEST(EmptyStopActionTest, StopSucceeds) { EmptyStopAction empty_stop_action; - const Result result = empty_stop_action.stop(cpp::stop_token{}, EmptyHandle{}); + const Result result = empty_stop_action.stop(cpp::stop_token{}, EmptyHandle{}); ASSERT_TRUE(result.has_value()); - EXPECT_EQ(result.value(), IComponent::RequestState::kSuccess); + EXPECT_EQ(result.value(), IComponent::RequestResult::kSuccess); } } // namespace diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/stop_action/istop_action.hpp b/score/launch_manager/src/daemon/src/process_group_manager/details/stop_action/istop_action.hpp index 1e9e91bc73..22870a7790 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/stop_action/istop_action.hpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/stop_action/istop_action.hpp @@ -32,7 +32,7 @@ class IStopAction /// @param stop_token Token which can be used to interrupt the action. /// @param handle The resource to act upon. /// @return Whether the action was successful, or failed with an error. - virtual Result stop(cpp::stop_token stop_token, const Handle& handle) const = 0; + virtual Result stop(cpp::stop_token stop_token, const Handle& handle) const = 0; }; } // namespace score::mw::lifecycle::internal diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/stop_action/mock_stop_action.hpp b/score/launch_manager/src/daemon/src/process_group_manager/details/stop_action/mock_stop_action.hpp index 9a3488ac24..6df0291150 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/stop_action/mock_stop_action.hpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/stop_action/mock_stop_action.hpp @@ -23,7 +23,7 @@ namespace score::mw::lifecycle::internal class MockStopAction : public IStopAction { public: - MOCK_METHOD(Result, stop, (cpp::stop_token, const Handle& handle), (override, const)); + MOCK_METHOD(Result, stop, (cpp::stop_token, const Handle& handle), (override, const)); }; } // namespace score::mw::lifecycle::internal diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/stop_action/process_stop_action.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/stop_action/process_stop_action.cpp index 680fd41250..5595580f4f 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/stop_action/process_stop_action.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/stop_action/process_stop_action.cpp @@ -22,7 +22,7 @@ ProcessStopAction::ProcessStopAction(osal::IProcess& launcher) : launcher_(launc { } -Result ProcessStopAction::stop(cpp::stop_token stop_token, const Handle& handle) const +Result ProcessStopAction::stop(cpp::stop_token stop_token, const Handle& handle) const { const ProcessHandle* const process = std::get_if(&handle); SCORE_LANGUAGE_FUTURECPP_ASSERT_MESSAGE( @@ -30,14 +30,14 @@ Result ProcessStopAction::stop(cpp::stop_token stop_to if (process->pid == -1) { - return IComponent::RequestState::kSuccess; + return IComponent::RequestResult::kSuccess; } const osal::OsalReturnType result = launcher_.requestTermination(process->pid); if (result == osal::OsalReturnType::kSuccess) { - return IComponent::RequestState::kWaiting; + return IComponent::RequestResult::kSuccess; } return MakeUnexpected(ExecErrc::kGeneralError); diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/stop_action/process_stop_action.hpp b/score/launch_manager/src/daemon/src/process_group_manager/details/stop_action/process_stop_action.hpp index 4e76ad72c1..713e8e9ad3 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/stop_action/process_stop_action.hpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/stop_action/process_stop_action.hpp @@ -35,7 +35,7 @@ class ProcessStopAction final : public IStopAction /// @param stop_token Token which can be used to interrupt the action. /// @param handle The resource to act upon. /// @return Whether the action was successful, or failed with an error. - Result stop(cpp::stop_token stop_token, const Handle& handle) const override; + Result stop(cpp::stop_token stop_token, const Handle& handle) const override; private: /// @brief The process launcher used to stop the process. diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/stop_action/process_stop_action_UT.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/stop_action/process_stop_action_UT.cpp index 2a4bd66215..35a347b811 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/stop_action/process_stop_action_UT.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/stop_action/process_stop_action_UT.cpp @@ -36,17 +36,17 @@ TEST(ProcessStopActionTest, StopCallsRequestTerminationWithPid) static_cast(process_stop_action.stop(cpp::stop_token{}, mock_handle)); } -TEST(ProcessStopActionTest, StopSucceedsReturnsWaiting) +TEST(ProcessStopActionTest, StopSucceedsReturnsSuccess) { NiceMock launcher; const ProcessStopAction process_stop_action(launcher); ON_CALL(launcher, requestTermination(_)).WillByDefault(Return(osal::OsalReturnType::kSuccess)); - const Result result = process_stop_action.stop(cpp::stop_token{}, mock_handle); + const Result result = process_stop_action.stop(cpp::stop_token{}, mock_handle); ASSERT_TRUE(result.has_value()); - EXPECT_EQ(result.value(), IComponent::RequestState::kWaiting); + EXPECT_EQ(result.value(), IComponent::RequestResult::kSuccess); } TEST(ProcessStopActionTest, StopWithInvalidatedHandleReturnsSuccess) @@ -56,10 +56,10 @@ TEST(ProcessStopActionTest, StopWithInvalidatedHandleReturnsSuccess) ProcessHandle handle = mock_handle; handle.pid = -1; - const Result result = process_stop_action.stop(cpp::stop_token{}, handle); + const Result result = process_stop_action.stop(cpp::stop_token{}, handle); ASSERT_TRUE(result.has_value()); - EXPECT_EQ(result.value(), IComponent::RequestState::kSuccess); + EXPECT_EQ(result.value(), IComponent::RequestResult::kSuccess); } TEST(ProcessStopActionTest, StopWithInvalidatedHandleDoesNotCallLauncher) @@ -79,7 +79,7 @@ TEST(ProcessStopActionTest, StopFailsReturnsError) ON_CALL(launcher, requestTermination(_)).WillByDefault(Return(osal::OsalReturnType::kFail)); - const Result result = process_stop_action.stop(cpp::stop_token{}, mock_handle); + const Result result = process_stop_action.stop(cpp::stop_token{}, mock_handle); EXPECT_FALSE(result.has_value()); EXPECT_EQ(result.error(), ExecErrc::kGeneralError); diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/transition_UT.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/transition_UT.cpp index 0fb818895b..40cff60611 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/transition_UT.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/transition_UT.cpp @@ -40,7 +40,7 @@ class MockComponent : public IComponent MOCK_METHOD(RequestResult, activate, (score::cpp::stop_token), (override)); MOCK_METHOD(RequestResult, deactivate, (score::cpp::stop_token), (override)); - MOCK_METHOD(RequestResult, tryHandleTermination, (int32_t), (override)); + MOCK_METHOD(TerminationResult, tryHandleTermination, (int32_t), (override)); MOCK_METHOD(IdentifierHash, getIdentifier, (), (const, override)); MOCK_METHOD(bool, active, (), (const, override));