Update from code review

src/lib/installation/clients/inst_update_installer.rb

@@ -173,14 +173,13 @@ def update_flag_file
     # @return [Boolean] true if installer was updated; false otherwise.
     def update_installer
       log.info("Adding update from #{self_update_url}")
-      if updates_manager.add_repository(self_update_url)
+      updates_manager.add_repository(self_update_url)
+      updated = updates_manager.has_repositories?
+      if updated
         log.info("Applying installer updates")
         updates_manager.apply_all
-        true
-      else
-        log.info("No packages were found")
-        false
       end
+      updated
 
     rescue ::Installation::UpdatesManager::NotValidRepo
       if !using_default_url?

src/lib/installation/updates_manager.rb

@@ -81,13 +81,13 @@ def initialize(duds_paths = DRIVER_UPDATES_PATHS)
     def add_repository(uri)
       new_repository = Installation::UpdateRepository.new(uri)
       new_repository.fetch
-      if new_repository.empty?
-        log.info("Update repository at #{uri} is empty. Skipping...")
-        false
+      has_packages = !new_repository.empty?
+      if has_packages
+        repositories << new_repository
       else
-        @repositories << new_repository
-        true
+        log.info("Update repository at #{uri} is empty. Skipping...")
       end
+      has_packages
     rescue Installation::UpdateRepository::NotValidRepo
       log.warn("Update repository at #{uri} could not be found")
       raise NotValidRepo
@@ -111,5 +111,10 @@ def apply_all
       (repositories + driver_updates).each(&:apply)
       repositories.each(&:cleanup)
     end
+
+    # Determines whether the manager has repositories with updates
+    def has_repositories?
+      !repositories.empty?
+    end
   end
 end

test/inst_update_installer_test.rb

@@ -9,13 +9,17 @@
   Yast.import "GetInstArgs"
   Yast.import "UI"
 
-  let(:manager) { double("update_manager", all_signed?: all_signed?, apply_all: true) }
+  let(:manager) do
+    double("update_manager", all_signed?: all_signed?, apply_all: true,
+      has_repositories?: has_repos)
+  end
   let(:url) { "http://update.opensuse.org/\$arch/update.dud" }
   let(:real_url) { "http://update.opensuse.org/#{arch}/update.dud" }
   let(:arch) { "x86_64" }
   let(:all_signed?) { true }
   let(:network_running) { true }
   let(:repo) { double("repo") }
+  let(:has_repos) { true }
 
   before do
     allow(Yast::Pkg).to receive(:GetArchitecture).and_return(arch)
@@ -79,6 +83,8 @@
       end
 
       context "when repository is empty" do
+        let(:has_repos) { false }
+
         it "does not restart YaST" do
           expect(manager).to receive(:add_repository)
             .and_return(false)

test/updates_manager_test.rb

@@ -12,8 +12,8 @@
 
   let(:uri) { URI("http://updates.opensuse.org/sles12") }
 
-  let(:repo0) { double("repo0", apply: true, cleanup: true, :empty? => false) }
-  let(:repo1) { double("repo1", apply: true, cleanup: true, :empty? => false) }
+  let(:repo0) { double("repo0", apply: true, cleanup: true, empty?: false) }
+  let(:repo1) { double("repo1", apply: true, cleanup: true, empty?: false) }
   let(:dud0)  { double("dud0", apply: true) }
 
   describe "#add_repository" do
@@ -125,4 +125,26 @@
       end
     end
   end
+
+  describe "#has_repositories?" do
+    context "when some repository was added" do
+      before do
+        allow(manager).to receive(:repositories).and_return([repo0])
+      end
+
+      it "returns true" do
+        expect(manager.has_repositories?).to eq(true)
+      end
+    end
+
+    context "when no repository was added" do
+      before do
+        allow(manager).to receive(:repositories).and_return([])
+      end
+
+      it "returns false" do
+        expect(manager.has_repositories?).to eq(false)
+      end
+    end
+  end
 end