LNDENG-636 (Shutdown/Reboot to DBus)

[mcw-work]

Changed shutdown manager to use dbus calls instead of direct executables.
Changed reboot logic in package changer to also use dbus.
Removed unit tests that use old shutdown logic.

[mcw-work]

Unit tests added for Shutdown/Restart logic. These don't test the dbus calls directly but ensure we are calling the necessary methods, or at least queuing them for the bus.

[Perfect5th]

Tiny inline comment.

Don't worry about the coverage failure. Codecov is being dumb and isn't comparing to the correct previous commit. If you push another commit it should be fine.

[Perfect5th]

Actually, looking at the coverage report was helpful. I assumed codecov was bad because it was using the wrong base commit, but there is also a real coverage issue.

Because you're patching _Reboot and the reactor, neither _Reboot nor _Shutdown ever actually get called by the tests, leading to coverage gaps. Here's what you likely want to do instead in your test class:

from unittest.mock import Mock, patch

...

    def setUp(self):
        super().setUp()
        ... # broker_service-related stuff
        self.dbus_mock = patch("landscape.client.manager.shutdownmanager.dbus").start()
        self.addCleanup(patch.stopall)

    def test_reboot(self):
        bus_object = Mock()
        self.dbus_mock.SystemBus.return_value = bus_object
        
        ... # actually run stuff, resulting in deferred

        def check(_):
            bus_object.get_object.assert_called_once()
            bus_object.Reboot.assert_called_once()  # or assert specific args, if you want to be safe

        return deferred

Similar for test_shutdown.

[mcw-work]

Thanks Mitch, tests have been updated to mock at the lower layer as recommended. Seems much neater to me as well. For the shutdown test I had to add a clock to ensure the delayed call got triggered. That needed a small tweak to the actual shutdown code but nothing too disconcerting. Hopefully covcheck will pass now.

[Perfect5th]

Apologies. Forgot to mention in my prev comment that the check callback needs to be added to the deferred:

deferred.addCallback(check)
return deferred

[mcw-work]

Tests seem to be sensible and full coverage now.

[Perfect5th]

LGTM, good work!