Update from code review

  * Add more logs to ease the debug
  * Make `rescue` statements more specific, catching only
    `SystemCallError` exceptions

    More info: https://blog.appsignal.com/2018/04/10/rescuing-exceptions-in-ruby.html

  * Use `String#tr` instead `String#gsub`

src/modules/Update.rb

@@ -811,15 +811,15 @@ def clean_backup
 
     # Restores the available backup(s)
     #
-    # If the VERSION_ID in the backup os-release file matches with the value stored in
+    # If the ID and VERSION_ID in the backup os-release file matches with the values stored in
     # /etc/os-release (see bsc#1097297), the available backup scripts (restores-*.sh) will be
     # executed **in the expected order** (see bsc#1089643).
     #
     # @see #restore_backup?
     def restore_backup
-      if restore_backup?
-        log.info "Restoring backup"
+      log.info "Restoring the backup"
 
+      if restore_backup?
         mounted_root = Installation.destdir
         available_restore_scripts = ::Dir.glob(File.join(mounted_root, BACKUP_DIR, "restore-*.sh"))
 
@@ -838,10 +838,14 @@ def restore_backup
     #
     # @see #version_from
     #
-    # @return [Boolean] true if update and backup have the same VERSION_ID; false otherwise
+    # @return [Boolean] true when versions matches; false otherwise
     def restore_backup?
       current_release = Pathname.new("#{Installation.destdir}/etc/os-release")
-      backed_release = Pathname.new("#{Installation.destdir}/#{BACKUP_DIR}/os-release")
+      backed_release  = Pathname.new("#{Installation.destdir}/#{BACKUP_DIR}/os-release")
+      current_version = version_from(current_release)
+      backed_version  = version_from(backed_release)
+
+      log.info "Version expected: #{current_version}. Backup version: #{backed_version}"
 
       version_from(current_release) == version_from(backed_release)
     end
@@ -855,11 +859,11 @@ def restore_backup?
     # @return [String] a string holding the ID and VERSION_ID or empty if something went wrong
     def version_from(file)
       content = file.read
-      id = content[/^ID=.*/].split("=", 2).last
-      version_id = content[/^VERSION_ID=.*/].split("=", 2).last
+      id = content[/^ID=.*/].split("=", 2).last.tr('"', '')
+      version_id = content[/^VERSION_ID=.*/].split("=", 2).last.tr('"', '')
 
       "#{id}-#{version_id}"
-    rescue
+    rescue SystemCallError
       ""
     end
 
@@ -907,7 +911,7 @@ def copy_os_release
         Pathname.new("#{Installation.destdir}/etc/os-release"),
         Pathname.new("#{Installation.destdir}/#{BACKUP_DIR}")
       )
-    rescue
+    rescue SystemCallError
       nil
     end
 

test/update_test.rb

@@ -84,6 +84,12 @@ def default_SetDesktopPattern_stubs
       Yast::Update.create_backup('testing', [])
     end
 
+    it "does not crash when os-release file does not exists" do
+      allow(::FileUtils).to receive(:cp).and_raise(Errno::ENOENT)
+
+      expect { Yast::Update.create_backup('testing', []) }.to_not raise_error
+    end
+
     it "create tarball including given name with all paths added" do
       name = "test-backup"
       paths = ["a", "b"]
@@ -157,6 +163,8 @@ def default_SetDesktopPattern_stubs
     let(:os_backup_release_content) { File.new("#{DATA_DIR}/etc/tw-os-release").read }
 
     before do
+      allow(Yast::Update.log).to receive(:info).and_call_original
+
       allow(::Dir).to receive(:glob).and_return(["restore-a.sh", "restore-b.sh"])
 
       allow(Pathname).to receive(:new)
@@ -178,6 +186,14 @@ def default_SetDesktopPattern_stubs
       Yast::Update.restore_backup
     end
 
+    context "when any file does not exists" do
+      it "does not crash" do
+        allow(os_backup_release_pathname).to receive(:read).and_raise(Errno::ENOENT)
+
+        expect { Yast::Update.restore_backup }.to_not raise_error
+      end
+    end
+
     context "when the release info match" do
       let(:os_backup_release_content) { os_release_content }
 
@@ -192,8 +208,11 @@ def default_SetDesktopPattern_stubs
     end
 
     context "when the release info does not match" do
-      it "logs an error" do
-        expect(Yast::Update.log).to receive(:error).with(/not restored/)
+      it "logs info and error" do
+        expect(Yast::Update.log).to receive(:info)
+          .with("Version expected: opensuse-leap-15.0. Backup version: opensuse-tumbleweed-20180911")
+          .and_call_original
+        expect(Yast::Update.log).to receive(:error).with(/not restored/).and_call_original
 
         Yast::Update.restore_backup
       end