make rubocop happy and changes from review

Co-authored-by: Martin Vidner <mvidner@suse.cz>

.rubocop.yml

@@ -100,7 +100,7 @@ Metrics/BlockNesting:
 # Offense count: 1
 # Configuration parameters: CountComments, CountAsOne.
 Metrics/ClassLength:
-  Max: 1282
+  Max: 1283
 
 # Offense count: 21
 # Configuration parameters: IgnoredMethods.

src/lib/y2iscsi_client/timeout_process.rb

@@ -3,29 +3,31 @@
 
 Yast.import "Popup"
 
+# yast2 iscsi client specific namespace
 module Y2IscsiClient
+  # Runner class that execute command with given timeout
   module TimeoutProcess
     include Yast::I18n
     extend Yast::I18n
 
     # @param [Array<String>] command as list of arguments
-    # @param [Integer] timeout in seconds
-    # @return [Array<Boolean, Array<String>] return pair of boolean if command
+    # @param [Integer] seconds timeout for command
+    # @return [Array(Boolean, Array<String>)] return pair of boolean if command
     #   succeed and stdout lines without ending newline
-    def self.run(command, timeout: 10)
+    def self.run(command, seconds: 10)
       textdomain "iscsi-client"
 
       # pass kill-after to ensure that command really dies even if ignore TERM
       stdout, stderr, exit_status = Yast::Execute.on_target!(
-        "/usr/bin/timeout", "--kill-after", "5s", "#{timeout}s",
+        "/usr/bin/timeout", "--kill-after=5s", "#{seconds}s",
         *command, stdout: :capture, stderr: :capture,
         allowed_exitstatus: 0..255, env: { "LC_ALL" => "POSIX" }
       )
 
       output = stdout.split("\n")
-      case(exit_status)
+      case exit_status
       when 0 then [true, output]
-      when 124, ( 128 + 9 )
+      when 124, (128 + 9)
         Yast::Popup.Error(_("Command timed out"))
         [false, output]
       else

src/modules/IscsiClientLib.rb

@@ -1496,6 +1496,7 @@ def CallConfigScript
       nil
     end
 
+    # @return [Array<String>]
     def GetDiscoveryCmd(ip, port, use_fw: false, only_new: false)
       Builtins.y2milestone("GetDiscoveryCmd ip:%1 port:%2 fw:%3 only new:%4",
         ip, port, use_fw, only_new)
@@ -1576,7 +1577,6 @@ def load_modules
     publish :function => :GetOffloadItems, :type => "list <term> ()"
     publish :function => :GetOffloadModules, :type => "list <string> ()"
     publish :function => :LoadOffloadModules, :type => "list <string> ()"
-    publish :function => :GetDiscoveryCmd, :type => "string (string, string, map)"
     publish :function => :getCurrentNodeValues, :type => "map <string, any> ()"
     publish :function => :iBFT?, :type => "boolean (map <string, any>)"
 

test/y2iscsi_client/timeout_process_test.rb

@@ -16,26 +16,31 @@
     end
 
     context "when command failed" do
-      it "shows error popup" do
-        expect(Yast::Popup).to receive(:Error)
+      # a command that produces stdout AND stderr AND fails
+      let(:command) { ["sh", "-c", "echo Copying data; echo >&2 Giving up; false"] }
+      it "shows error popup with its stderr" do
+        expect(Yast::Popup).to receive(:Error).with("Giving up\n")
 
-        described_class.run(["false"])
+        described_class.run(command)
       end
 
-      it "returns false and its stderr" do
-        expect(described_class.run(["false"])).to eq([false, []])
+      it "returns false and its stdout" do
+        expect(described_class.run(command)).to eq([false, ["Copying data"]])
       end
     end
 
     context "when command runs after timeout" do
-      it "shows error popup" do
+      # a command that produces stdout AND stderr AND takes a long time
+      let(:command) { ["sh", "-c", "echo Copying data; echo >&2 Mars is too far; sleep 999"] }
+
+      it "shows generic error popup" do
         expect(Yast::Popup).to receive(:Error).with("Command timed out")
 
-        described_class.run(["sleep", "10"], timeout: 1)
+        described_class.run(command, seconds: 1)
       end
 
-      it "returns false and its stderr" do
-        expect(described_class.run(["sleep", "10"], timeout: 1)).to eq([false, []])
+      it "returns false and its stdout" do
+        expect(described_class.run(command, seconds: 1)).to eq([false, ["Copying data"]])
       end
     end
   end