Code review fixes

- use Yast::Execute for running the commands

src/lib/installation/instsys_cleaner.rb

@@ -16,13 +16,12 @@
 #
 
 require "fileutils"
-require "shellwords"
-require "English"
 
 require "yast"
 require "yast/logger"
 # memory size detection
 require "yast2/hw_detection"
+require "yast2/execute"
 
 module Installation
   class InstsysCleaner
@@ -93,53 +92,60 @@ def self.unmount_kernel_modules
       log.info("Removing the kernel modules inst-sys image")
       log_space_usage("Before removing the kernel modules:")
 
-      # find the loop device for the mount point
-      mounts = `mount`.split("\n")
-      mounts.find { |m| m.match(/\A(\/dev\/loop.*) on #{Regexp.escape(KERNEL_MODULES_MOUNT_POINT)} /) }
+      # find the image for the mount point
+      mount_point = find_device || return
+      image = losetup_backing_file(mount_point) || return
+
+      # unmount the image
+      Yast::Execute.locally("umount", image, allowed_exitstatus: 1..255)
+
+      # remove the loop device binding
+      Yast::Execute.locally("losetup", "-d", image, allowed_exitstatus: 1..255)
+
+      # remove the image file
+      ::FileUtils.rm_rf(image)
+
+      log_space_usage("After removing the kernel modules:")
+    end
 
+    # Log the memory and disk usage to see how the clean up was effective
+    def self.log_space_usage(msg)
+      log.info(msg)
+      # the Cheetah backend by default logs the output
+      Yast::Execute.locally("df", "-m")
+      Yast::Execute.locally("free", "-m")
+    end
+
+    # Find the device for the kernel modules mount point.
+    # @return [String,nil] device name (/dev/loopN) or nil if not found
+    def self.find_device
+      mounts = Yast::Execute.locally("mount", stdout: :capture).split("\n")
+      mounts.find { |m| m.match(/\A(\/dev\/loop.*) on #{Regexp.escape(KERNEL_MODULES_MOUNT_POINT)} /) }
       device = Regexp.last_match(1)
+
       if !device
         log.warn("Cannot find the loop device for the #{KERNEL_MODULES_MOUNT_POINT} mount point")
-        return
       end
 
+      device
+    end
+
+    # Find the backend file for a loop device.
+    # @param device [String] device name
+    # @return [String,nil] backing file or nil if not found
+    def self.losetup_backing_file(device)
       # find the backend file for the loop device
-      file = `losetup -n -O BACK-FILE #{Shellwords.escape(device)}`.strip
+      file = Yast::Execute.locally("losetup", "-n", "-O", "BACK-FILE", device, stdout: :capture).strip
 
       if file.nil? || file.empty?
         log.warn("Cannot find the backend file for the #{device} device")
-        return
-      end
-
-      # unmount the loop device
-      `umount #{KERNEL_MODULES_MOUNT_POINT}`
-      if $CHILD_STATUS && !$CHILD_STATUS.success?
-        log.warn("Unmouting #{KERNEL_MODULES_MOUNT_POINT} failed")
-        return
-      end
-
-      # remove the loop device binding
-      `losetup -d #{Shellwords.escape(device)}`
-      if $CHILD_STATUS && !$CHILD_STATUS.success?
-        log.warn("Detaching loopback device #{device} failed")
-        return
+        return nil
       end
 
-      # remove the image file
-      ::FileUtils.rm_rf(file)
-
-      log_space_usage("After removing the kernel modules:")
-    end
-
-    def self.log_space_usage(msg)
-      log.info(msg)
-      log.info("disk usage in MB ('df -m'):")
-      log.info(`df -m`)
-      log.info("memory usage in MB ('free -m'):")
-      log.info(`free -m`)
+      file
     end
 
     private_class_method :log_space_usage, :unmount_kernel_modules,
-      :cleanup_zypp_cache
+      :cleanup_zypp_cache, :find_device, :losetup_backing_file
   end
 end

test/helpers.rb

@@ -14,7 +14,7 @@ def fixtures_dir(*dirs)
   end
 
   # Read the fixture file
-  # @param *path [Array<String>] path components
+  # @param path [Array<String>] path components
   # @see fixtures_dir
   # @return [String] the loaded file
   def load_fixture(*path)

test/instsys_cleaner_test.rb

@@ -15,8 +15,8 @@ def stub_logging
         expect(Yast::Mode).to receive(:installation).and_return(true)
 
         # mock the logged memory stats
-        allow(subject.class).to receive(:`).with("df -m").and_return("")
-        allow(subject.class).to receive(:`).with("free -m").and_return("")
+        allow(Yast::Execute).to receive(:locally).with("df", "-m").and_return("")
+        allow(Yast::Execute).to receive(:locally).with("free", "-m").and_return("")
       end
 
       it "removes the libzypp cache if the memory less than 640MB" do
@@ -36,11 +36,11 @@ def stub_logging
 
         # the order of the executed commands is important, check it explicitly
         expect(File).to receive(:exist?).with("/parts/mp_0000/lib/modules").and_return(true).ordered
-        expect(subject.class).to receive(:`).with("mount").and_return(load_fixture("inst-sys", "mount.out")).ordered
-        expect(subject.class).to receive(:`).with("losetup -n -O BACK-FILE /dev/loop0")
+        expect(Yast::Execute).to receive(:locally).with("mount", stdout: :capture).and_return(load_fixture("inst-sys", "mount.out")).ordered
+        expect(Yast::Execute).to receive(:locally).with("losetup", "-n", "-O", "BACK-FILE", "/dev/loop0", stdout: :capture)
           .and_return(load_fixture("inst-sys", "losetup.out")).ordered
-        expect(subject.class).to receive(:`).with("umount /parts/mp_0000").ordered
-        expect(subject.class).to receive(:`).with("losetup -d /dev/loop0").ordered
+        expect(Yast::Execute).to receive(:locally).with("umount", "/parts/mp_0000").ordered
+        expect(Yast::Execute).to receive(:locally).with("losetup", "-d", "/dev/loop0").ordered
         expect(FileUtils).to receive(:rm_rf).with("/parts/00_lib").ordered
 
         subject.class.make_clean