Update from code review

src/lib/y2packager/release_notes_fetchers/rpm.rb

@@ -30,6 +30,9 @@ module ReleaseNotesFetchers
     # "release-notes()" for the given product. For instance, a package which provides
     # "release-notes() = SLES" will provide release notes for the SLES product.
     #
+    # If more than one product provide release notes for that product, the first
+    # one in alphabetical order will be selected.
+    #
     # This reader takes care of downloading the release notes package (if any),
     # extracting its content and returning release notes for a given language/format.
     #
@@ -80,13 +83,15 @@ def latest_version
       #
       # This method queries libzypp asking for the package which contains release
       # notes for the given product. It relies on the `release-notes()` tag.
+      # If more than one product provide release notes for that product, the first
+      # one in alphabetical order will be selected.
       #
       # @return [Package,nil] Package containing the release notes; nil if not found
       def release_notes_package
         return @release_notes_package if @release_notes_package
         provides = Yast::Pkg.PkgQueryProvides("release-notes()")
         release_notes_packages = provides.map(&:first).uniq
-        package_name = release_notes_packages.find do |name|
+        package_name = release_notes_packages.sort.find do |name|
           dependencies = Yast::Pkg.ResolvableDependencies(name, :package, "").first["deps"]
           dependencies.any? do |dep|
             dep["provides"].to_s.match(/release-notes\(\)\s*=\s*#{product.name}\s*/)

src/lib/y2packager/release_notes_fetchers/url.rb

@@ -313,9 +313,9 @@ def download_release_notes_index(url_base)
             log.info("Release notes index empty, not filtering further downloads")
             nil
           else
-            rn_filter = index_file.split("\n")
-            log.info("Index of RN files at the server: #{rn_filter}")
-            rn_filter
+            rn_index = index_file.split("\n")
+            log.info("Index of RN files at the server: #{rn_index}")
+            rn_index
           end
         elsif ok.nil?
           nil
@@ -346,7 +346,8 @@ def curl_download(url, filename, max_time: 300)
           max_time
         )
         ret = Yast::SCR.Execute(Yast::Path.new(".target.bash"), cmd)
-        log.info("#{cmd} returned #{ret}")
+        filtered_cmd = cmd.sub(/--proxy-user '[^']*'/, "--proxy-user @PROXYPASSWORD@")
+        log.info("#{filtered_cmd} returned #{ret}")
         reason = CURL_GIVE_UP_RETURN_CODES[ret]
         if reason
           log.info "Communication with server failed (#{reason}), skipping further attempts."

src/lib/y2packager/release_notes_reader.rb

@@ -33,9 +33,9 @@ module Y2Packager
   # We can distinguish two different case:
   #
   # * When the system *is registered*: release notes will be obtained from RPM packages.
-  #   If release notes are not found there, it will fall back to the relnotes_url property.
-  #   This behaviour covers the case in which you are installing behind a SMT but without
-  #   access to Internet.
+  #   If release notes are not found there, it will fall back to the
+  #   "relnotes_url" product property.  This behaviour covers the case in which
+  #   you are installing behind a SMT but without access to Internet.
   # * When the system *is not registered*: it will work the other way around, trying
   #   first relnotes_url and falling back to RPM packages.
   #
@@ -48,7 +48,7 @@ module Y2Packager
   # or the stored version is outdated (maybe a new package is now available), it will
   # try to get that version.
   #
-  # Take into account that, when using the relnotes_url property, a URL that already
+  # Take into account that, when using the relnotes_url property, an URL that already
   # failed will not be retried again. See ReleaseNotesFetchers::Url for further details.
   class ReleaseNotesReader
     include Yast::Logger

test/lib/release_notes_fetchers/rpm_test.rb

@@ -95,7 +95,24 @@
       end
     end
 
-    context "when there is more than one package" do
+    context "when more than one package provides release notes" do
+      let(:provides) do
+        [
+          ["release-notes-more-dummy", :CAND, :NONE],
+          ["release-notes-dummy", :CAND, :NONE]
+        ]
+      end
+
+      it "uses the first in alphabetical order" do
+        expect(Y2Packager::Package).to receive(:find)
+          .with("release-notes-dummy").and_return([package])
+        expect(Y2Packager::Package).to_not receive(:find)
+          .with("release-notes-more-dummy")
+        fetcher.release_notes(prefs)
+      end
+    end
+
+    context "when there is more than one package version" do
       let(:other_package) { Y2Packager::Package.new("release-notes-dummy", 2, "15.0") }
       let(:packages) { [other_package, package] }
 

test/lib/release_notes_fetchers/url_test.rb

@@ -246,6 +246,7 @@
         it "uses an unauthenticated proxy" do
           expect(Yast::SCR).to receive(:Execute) do |_path, cmd|
             expect(cmd).to include("--proxy http://proxy.example.com")
+            expect(cmd).to_not include("--proxy-user")
           end
 
           fetcher.release_notes(prefs)

test/lib/release_notes_reader_test.rb

@@ -100,9 +100,9 @@
 
       it "retrieves release notes from external sources" do
         expect(url_reader).to receive(:release_notes).with(
-          Y2Packager::ReleaseNotesContentPrefs.new("cz_CZ", "en", :txt)
+          Y2Packager::ReleaseNotesContentPrefs.new("cs_CZ", "en", :txt)
         )
-        rn = reader.release_notes(user_lang: "cz_CZ", format: :txt)
+        rn = reader.release_notes(user_lang: "cs_CZ", format: :txt)
         expect(rn).to eq(relnotes_from_url)
       end
 
@@ -111,9 +111,9 @@
 
         it "tries to get release notes from RPM packages" do
           expect(url_reader).to receive(:release_notes).with(
-            Y2Packager::ReleaseNotesContentPrefs.new("cz_CZ", "en", :html)
+            Y2Packager::ReleaseNotesContentPrefs.new("cs_CZ", "en", :html)
           )
-          rn = reader.release_notes(user_lang: "cz_CZ", format: :html)
+          rn = reader.release_notes(user_lang: "cs_CZ", format: :html)
           expect(rn).to eq(release_notes)
         end
       end

test/lib/release_notes_test.rb

@@ -24,7 +24,7 @@
 
     context "when language does not match" do
       it "returns false" do
-        expect(rn.matches?("cz_CZ", format, version)).to eq(false)
+        expect(rn.matches?("cs_CZ", format, version)).to eq(false)
       end
     end