Add error handling to SystemService#errors

library/system/src/lib/yast2/system_service.rb

@@ -49,9 +49,13 @@ module Yast2
   class SystemService
     extend Forwardable
 
-    # @return [Yast::SystemdService]
+    # @return [Yast::SystemdServiceClass::Service]
     attr_reader :service
 
+    # @return [Hash<Symbol,Object>] Errors when trying to write changes to the
+    #   underlying system.
+    attr_reader :errors
+
     def_delegators :@service, :running?, :start, :stop, :restart, :active?,
       :active_state, :sub_state, :name, :description
 
@@ -80,6 +84,7 @@ def find_many(names)
     def initialize(service)
       @service = service
       @changes = {}
+      @errors = {}
     end
 
     # Returns the start mode
@@ -136,6 +141,7 @@ def active
     #
     # @param set_status [Boolean] Do not change service status. Useful when running on 1st stage.
     def save(ignore_status: false)
+      clear_errors
       save_start_mode
       set_current_status unless ignore_status
       reload
@@ -188,26 +194,42 @@ def current_start_mode
     # Sets start mode to the underlying system
     def save_start_mode
       return unless changes[:start_mode]
-      case changes[:start_mode]
-      when :on_boot
-        service.enable
-        socket.disable
-      when :on_demand
-        service.disable
-        socket.enable
-      when :manual
-        service.disable
-        socket.disable
-      end
+      result =
+        case changes[:start_mode]
+        when :on_boot
+          service.enable && socket.disable
+        when :on_demand
+          service.disable && socket.enable
+        when :manual
+          service.disable && socket.disable
+        end
+      register_error(:start_mode) unless result
     end
 
     # Sets service status
     def set_current_status
-      if changes[:active] && !service.active?
-        service.start
-      elsif changes[:active] == false && service.active?
-        service.stop
-      end
+      return if changes[:active].nil?
+      result =
+        if changes[:active] && !service.active?
+          service.start
+        elsif changes[:active] == false && service.active?
+          service.stop
+        end
+      register_error(:active) if result == false
+    end
+
+    # Registers error information
+    #
+    # Stores the source of error and the value which caused it.
+    #
+    # @param key [Symbol] Source of error
+    def register_error(key)
+      errors[key] = changes[key]
+    end
+
+    # Clears registered errors
+    def clear_errors
+      @errors.clear
     end
 
     # Returns the associated socket

library/system/test/yast2/system_service_test.rb

@@ -238,8 +238,8 @@
       let(:start_mode) { :on_boot }
 
       it "enables the service to start on boot" do
-        expect(service).to receive(:enable)
-        expect(socket).to receive(:disable)
+        expect(service).to receive(:enable).and_return(true)
+        expect(socket).to receive(:disable).and_return(true)
         system_service.save
       end
     end
@@ -248,8 +248,8 @@
       let(:start_mode) { :on_demand }
 
       it "enables the socket" do
-        expect(service).to receive(:disable)
-        expect(socket).to receive(:enable)
+        expect(service).to receive(:disable).and_return(true)
+        expect(socket).to receive(:enable).and_return(true)
         system_service.save
       end
     end
@@ -258,12 +258,25 @@
       let(:start_mode) { :manual }
 
       it "disables the service and the socket" do
-        expect(service).to receive(:disable)
-        expect(socket).to receive(:disable)
+        expect(service).to receive(:disable).and_return(true)
+        expect(socket).to receive(:disable).and_return(true)
         system_service.save
       end
     end
 
+    context "when setting the start_mode fails" do
+      let(:start_mode) { :on_demand }
+
+      before do
+        allow(service).to receive(:disable).and_return(false)
+      end
+
+      it "registers the error" do
+        system_service.save
+        expect(system_service.errors).to eq(start_mode: :on_demand)
+      end
+    end
+
     context "when active is set to true" do
       before { system_service.active = true }
 
@@ -280,13 +293,34 @@
         let(:active?) { false }
 
         it "tries to activate the service" do
-          expect(service).to receive(:start)
+          expect(service).to receive(:start).and_return(true)
           system_service.save
         end
 
-        it "does not active the service if the status must be ignored" do
-          expect(service).to_not receive(:start)
-          system_service.save(ignore_status: true)
+        context "and the service is successfully started" do
+          it "does not register any error" do
+            allow(service).to receive(:start).and_return(true)
+            system_service.save
+            expect(system_service.errors).to_not have_key(:activate)
+          end
+        end
+
+        context "and the service could not be started" do
+          before do
+            allow(service).to receive(:start).and_return(false)
+          end
+
+          it "registers the error" do
+            system_service.save
+            expect(system_service.errors).to eq(active: true)
+          end
+        end
+
+        context "and the status must be ignored" do
+          it "does not try to activate the service" do
+            expect(service).to_not receive(:start)
+            system_service.save(ignore_status: true)
+          end
         end
       end
     end
@@ -302,13 +336,34 @@
           system_service.save
         end
 
-        it "does not stop the service if the status must be ignored" do
-          expect(service).to_not receive(:start)
-          system_service.save(ignore_status: true)
+        context "and the service is successfully stopped" do
+          it "does not register any error" do
+            allow(service).to receive(:stop).and_return(true)
+            system_service.save
+            expect(system_service.errors).to_not have_key(:activate)
+          end
+        end
+
+        context "and the service could not be stopped" do
+          before do
+            allow(service).to receive(:stop).and_return(false)
+          end
+
+          it "registers the error" do
+            system_service.save
+            expect(system_service.errors).to eq(active: false)
+          end
+        end
+
+        context "and the status must be ignored" do
+          it "does not try to stop the service" do
+            expect(service).to_not receive(:start)
+            system_service.save(ignore_status: true)
+          end
         end
       end
 
-      context "and the service is inactive" do
+      context "and the service is already inactive" do
         let(:active?) { false }
 
         it "does not try to stop the service again" do