diff --git a/plinth/app.py b/plinth/app.py index 5df311ba7..be936b674 100644 --- a/plinth/app.py +++ b/plinth/app.py @@ -211,7 +211,6 @@ class App: """Update the status of all follower components. Do not query or update the status of the leader components. - """ for component in self.components.values(): if not component.is_leader: @@ -230,7 +229,6 @@ class App: and then supplementing the results with any app level diagnostic tests. Also see :meth:`.has_diagnostics`. - """ results = [] for component in self.components.values(): @@ -255,7 +253,6 @@ class App: it is assumed that it is for implementing diagnostic tests and this method returns True for such an app. Override this method if this default behavior does not fit the needs. - """ # App implements some diagnostics if self.__class__.diagnose is not App.diagnose: @@ -268,6 +265,40 @@ class App: return False + def repair(self, failed_checks: _list_type) -> bool: + """Try to fix failed diagnostics. + + The default implementation asks relevant components to repair, and then + requests re-run setup for the app. + + failed_checks is a list of DiagnosticChecks that had failed or resulted + in a warning. The list will be split up by component_id, and passed to + the appropriate components. Remaining failed diagnostics do not have a + related component, and should be handled by the app. + + Returns whether the app setup should be re-run. + """ + should_rerun_setup = False + + # Group the failed_checks by component + components_failed_checks = collections.defaultdict(list) + for failed_check in failed_checks: + if failed_check.component_id: + components_failed_checks[failed_check.component_id].append( + failed_check) + else: + # There is a failed check with no related component. + should_rerun_setup = True + + # Repair each component that has failed checks + for component_id, component in self.components.items(): + if components_failed_checks[component_id]: + result = component.repair( + components_failed_checks[component_id]) + should_rerun_setup = should_rerun_setup or result + + return should_rerun_setup + class Component: """Interface for an app component. @@ -319,7 +350,6 @@ class Component: enumeration from 'failed', 'passed', 'error', 'warning' and 'not_done'. Also see :meth:`.has_diagnostics`. - """ return [] @@ -333,10 +363,25 @@ class Component: is assumed that it is for implementing diagnostic tests and this method returns True for such a component. Override this method if this default behavior does not fit the needs. - """ return self.__class__.diagnose is not Component.diagnose + def repair(self, failed_checks: list) -> bool: + """Try to fix failed diagnostics. + + The default implementation only requests re-run setup for the app. + + Returns whether the app setup should be re-run by the caller. + + This method should be overridden by components that implement + diagnose(), if there is a known way to fix failed checks. The return + value can be changed to False to avoid causing a re-run setup. + + failed_checks is a list of DiagnosticChecks related to this component + that had failed or warning result. + """ + return True + class FollowerComponent(Component): """Interface for an app component that follows other components. diff --git a/plinth/tests/test_app.py b/plinth/tests/test_app.py index 1ea6fc523..24c46033b 100644 --- a/plinth/tests/test_app.py +++ b/plinth/tests/test_app.py @@ -279,6 +279,38 @@ def test_app_has_diagnostics(app_with_components): assert app.has_diagnostics() +@patch('plinth.setup.run_setup_on_app') +def test_app_repair(_run_setup_on_app, app_with_components): + """Test running repair on an app.""" + component = app_with_components.get_component('test-follower-1') + component.repair = Mock(return_value=True) + + check1 = DiagnosticCheck('check1', 'check1', Result.FAILED, {}) + check2 = DiagnosticCheck('check2', 'check2', Result.WARNING, {}) + check3 = DiagnosticCheck('check3', 'check3', Result.FAILED, {}, + 'test-follower-1') + should_rerun_setup = app_with_components.repair([]) + assert not should_rerun_setup + + should_rerun_setup = app_with_components.repair([check1]) + assert should_rerun_setup + + should_rerun_setup = app_with_components.repair([check2]) + assert should_rerun_setup + + should_rerun_setup = app_with_components.repair([check1, check2]) + assert should_rerun_setup + component.repair.assert_not_called() + + should_rerun_setup = app_with_components.repair([check3]) + assert should_rerun_setup + assert component.repair.mock_calls == [call([check3])] + + component.repair = Mock(return_value=False) + should_rerun_setup = app_with_components.repair([check3]) + assert not should_rerun_setup + + def test_component_initialization(): """Test that component is initialized properly.""" with pytest.raises(ValueError): @@ -340,6 +372,13 @@ def test_component_has_diagnostics(): assert not component.has_diagnostics() +@patch('plinth.setup.run_setup_on_app') +def test_component_repair(_run_setup_on_app): + """Test running repair on component.""" + component = Component('test-component') + assert component.repair(['test-check']) + + def test_follower_component_initialization(): """Test that follower component is initialized properly.""" component = FollowerComponent('test-follower-1')