System calls with Open4 && more tests

Gemfile

@@ -36,6 +36,7 @@ gem 'bcrypt', '~> 3.1.7'
 # Use ActiveStorage variant
 # gem 'mini_magick', '~> 4.8'
 
+gem 'open4'
 gem 'naught'
 
 # Reduces boot times through caching; required in config/boot.rb

Gemfile.lock

@@ -167,6 +167,7 @@ GEM
     notiffany (0.1.1)
       nenv (~> 0.1)
       shellany (~> 0.0)
+    open4 (1.3.4)
     orm_adapter (0.5.0)
     parallel (1.12.1)
     parser (2.5.1.0)
@@ -319,6 +320,7 @@ DEPENDENCIES
   listen (>= 3.0.5, < 3.2)
   mysql2 (>= 0.4.4, < 0.6.0)
   naught
+  open4
   pry-byebug
   pry-rails
   puma (~> 3.11)

app/services/system_command.rb

@@ -1,5 +1,8 @@
+require "open4"
 require "null_logger"
 
+SystemCommandResult = Struct.new(:command, :exit_status, :stdout, :stderr)
+
 class SystemCommand
   attr_reader :program
   attr_reader :args
@@ -14,11 +17,11 @@ class SystemCommand
   def execute
     logger.log :before_command, syscmd
 
-    raw = `syscmd`
+    result = call(syscmd)
 
-    logger.log :after_command, raw
+    logger.log :after_command, result
 
-    raw
+    result
   end
 
   def syscmd
@@ -31,6 +34,18 @@ class SystemCommand
 
   private
 
+  def call(cmd)
+    pid, _, stdout, stderr = Open4.popen4 cmd
+    _, status = Process.waitpid2 pid
+
+    SystemCommandResult.new(
+      syscmd,
+      status.exitstatus,
+      stdout.read.strip,
+      stderr.read.strip,
+    )
+  end
+
   def escape_arg(arg)
     arg.to_s.gsub('"') { '\"' }
   end

app/services/whois.rb

@@ -1,33 +1,47 @@
 require "null_logger"
 require "domain_helper"
-require "whois/command"
+require "system_command"
-require "whois/parser"
+require_relative "whois/parser"
-require "whois/response"
+require_relative "whois/response"
+require_relative "whois/errors"
 
 module Whois
   class << self
-    def ask(domain, logger: NullLogger.new)
+    def ask(domain, system_klass: SystemCommand, logger: NullLogger.new)
-      Service.new(domain, logger).call
+      Service.new(domain, system_klass, logger: logger).call
     end
   end
 
   class Service
     attr_reader :domain
     attr_reader :logger
+    attr_reader :system_klass
 
-    def initialize(domain, logger)
+    def initialize(domain, system_klass: SystemCommand, logger: NullLogger.new)
       @domain = domain
       @logger = logger
+      @system_klass = system_klass
     end
 
     def call
-      command = Command.new(domain, logger: logger)
+      result = run_command
-      raw_response = command.run
+      parse(result)
+    end
 
+    def run_command
+      command = system_klass.new("whois", domain, logger: logger)
+      result = command.execute
+
+      unless result.exit_status.zero?
+        fail WhoisCommandError, "Whois command failed with status #{result.exit_status}"
+      end
+
+      result
+    end
+
+    def parse(result)
       parser = Parser.for(domain, logger: logger)
-      response = parser.parse(raw_response)
+      parser.parse(result.stdout)
-
-      response
     end
   end
 end

app/services/whois/command.rb

@@ -1,18 +0,0 @@
-require "null_logger"
-require "system_command"
-
-module Whois
-  class Command
-    attr_reader :logger
-    attr_reader :domain
-
-    def initialize(domain, logger: NullLogger.new)
-      @domain = domain
-      @logger = logger
-    end
-
-    def run
-      SystemCommand.new("whois", domain, logger: logger).execute
-    end
-  end
-end

app/services/whois/errors.rb

@@ -1,6 +1,10 @@
 module Whois
-  class UnsupportedDomainError < StandardError; end
+  class WhoisError < StandardError; end
-  class ParserError < StandardError; end
+
+  class WhoisCommandError < WhoisError; end
+  class UnsupportedDomainError < WhoisError; end
+  class ParserError < WhoisError; end
+
   class CommentNotFoundError < ParserError; end
   class FieldNotFoundError < ParserError; end
   class MissingDateFormatError < ParserError; end

test/services/system_command_test.rb

@@ -4,16 +4,23 @@ require "system_command"
 class SystemCommandTest < ActiveSupport::TestCase
   test "should execute and log a command" do
     mock_logger = Minitest::Mock.new
-    expected = 'whois "example.org"'
+    expected_cmd = 'whois "example.org"'
 
-    mock_logger.expect(:log, nil, [:before_command, expected])
+    expected_result = SystemCommandResult.new(
-    mock_logger.expect(:log, nil, [:after_command, "my result"])
+      expected_cmd,
+      0,
+      "my result",
+      "",
+    )
+
+    mock_logger.expect(:log, nil, [:before_command, expected_cmd])
+    mock_logger.expect(:log, nil, [:after_command, expected_result])
 
     command = SystemCommand.new("whois", "example.org", logger: mock_logger)
-    assert_equal expected, command.syscmd
+    assert_equal expected_cmd, command.syscmd
 
-    command.stub(:`, "my result") do
+    command.stub(:call, expected_result) do
-      assert_equal "my result", command.execute
+      assert_equal expected_result, command.execute
     end
 
     mock_logger.verify

test/services/whois/command_test.rb

@@ -1,27 +0,0 @@
-require "test_helper"
-require "whois/command"
-
-module Whois
-  class CommandTest < ActiveSupport::TestCase
-    test "should return the result and log the command" do
-      result = "mocked whois result"
-
-      mock = Minitest::Mock.new
-      mock.expect(:execute, result)
-
-      stub = lambda do |program, args, _logger|
-        assert_equal "whois", program
-        assert_equal "example.org", args
-
-        mock
-      end
-
-      SystemCommand.stub(:new, stub) do
-        command = Command.new("example.org")
-        assert_equal result, command.run
-      end
-
-      mock.verify
-    end
-  end
-end

test/services/whois_test.rb

@@ -1,9 +1,53 @@
 require "test_helper"
 require "whois"
+require "system_command"
 
-class WhoisTest < ActiveSupport::TestCase
+module Whois
-  test "should instanciate a parser class matching the tld" do
+  class ServiceTest < ActiveSupport::TestCase
-    # TODO: stub system command
+    test "should run the command, return the result" do
-    # assert_kind_of Whois::Response, Whois.ask("example.fr")
+      result = OpenStruct.new(exit_status: 0)
+
+      mock_system_klass("whois", "example.org", result) do |system_klass|
+        service = Service.new("example.org", system_klass: system_klass)
+        assert_equal result, service.run_command
+      end
+    end
+
+    test "should raise an exception if exit status > 0" do
+      result = OpenStruct.new(exit_status: 1)
+
+      mock_system_klass("whois", "example.org", result) do |system_klass|
+        service = Service.new("example.org", system_klass: system_klass)
+
+        assert_raises WhoisCommandError do
+          service.run_command
+        end
+      end
+    end
+
+    test "should parse from a command result" do
+      result = OpenStruct.new(
+        exit_status: 0,
+        stdout: file_fixture("whois/domain.fr.txt").read,
+      )
+
+      service = Service.new("domain.fr")
+      assert_kind_of Response, service.parse(result)
+    end
+
+    def mock_system_klass(program, command_args, result)
+      system_klass = Minitest::Mock.new
+      system_command = Minitest::Mock.new.expect(:execute, result)
+      system_klass.expect(:new, system_command) do |arg1, arg2, logger:|
+        arg1 == program &&
+          arg2 == command_args &&
+          logger.class == NullLogger
+      end
+
+      yield system_klass
+
+      system_klass.verify
+      system_command.verify
+    end
   end
 end