Simplify Hbc::SystemCommand::Result#plist.

Library/Homebrew/cask/lib/hbc/system_command.rb

@@ -137,48 +137,35 @@ module Hbc
         @exit_status = exit_status
       end
 
-      def plist
-        @plist ||= self.class._parse_plist(@command, @stdout.dup)
-      end
-
       def success?
         @exit_status.zero?
       end
 
-      def merged_output
+      def plist
-        @merged_output ||= @stdout + @stderr
+        @plist ||= begin
-      end
+          output = stdout
 
-      def to_s
+          if /\A(?<garbage>.*?)<\?\s*xml/m =~ output
-        @stdout
+            output = output.sub(/\A#{Regexp.escape(garbage)}/m, "")
-      end
+            warn_plist_garbage(garbage)
+          end
 
-      def self._warn_plist_garbage(command, garbage)
+          if %r{<\s*/\s*plist\s*>(?<garbage>.*?)\Z}m =~ output
-        return true unless garbage =~ /\S/
+            output = output.sub(/#{Regexp.escape(garbage)}\Z/, "")
-        external = File.basename(command.first)
+            warn_plist_garbage(garbage)
-        lines = garbage.strip.split("\n")
+          end
-        opoo "Non-XML stdout from #{external}:"
-        $stderr.puts lines.map { |l| "    #{l}" }
-      end
 
-      def self._parse_plist(command, output)
+          Plist.parse_xml(output)
-        raise CaskError, "Empty plist input" unless output =~ /\S/
-        output.sub!(/\A(.*?)(<\?\s*xml)/m, '\2')
-        _warn_plist_garbage(command, Regexp.last_match[1]) if ARGV.debug?
-        output.sub!(%r{(<\s*/\s*plist\s*>)(.*?)\Z}m, '\1')
-        _warn_plist_garbage(command, Regexp.last_match[2])
-        xml = Plist.parse_xml(output)
-        unless xml.respond_to?(:keys) && !xml.keys.empty?
-          raise CaskError, <<~EOS
-            Empty result parsing plist output from command.
-              command was:
-              #{command}
-              output we attempted to parse:
-              #{output}
-          EOS
         end
-        xml
       end
+
+      def warn_plist_garbage(garbage)
+        return unless ARGV.verbose?
+        return unless garbage =~ /\S/
+        opoo "Received non-XML output from #{Formatter.identifier(command.first)}:"
+        $stderr.puts garbage.strip
+      end
+      private :warn_plist_garbage
     end
   end
 end

Library/Homebrew/test/cask/system_command_result_spec.rb

@@ -1,10 +1,21 @@
 require "hbc/system_command"
 
 describe Hbc::SystemCommand::Result, :cask do
-  describe "::_parse_plist" do
+  describe "#plist" do
-    subject { described_class._parse_plist(command, input) }
+    subject { described_class.new(command, stdout, "", 0).plist }
 
-    let(:command) { Hbc::SystemCommand.new("/usr/bin/true", {}) }
+    let(:command) { ["/usr/bin/true"] }
+    let(:garbage) {
+      <<~EOS
+        Hello there! I am in no way XML am I?!?!
+
+          That's a little silly... you were expecting XML here!
+
+        What is a parser to do?
+
+        Hopefully <not> explode!
+      EOS
+    }
     let(:plist) {
       <<~EOS
         <?xml version="1.0" encoding="UTF-8"?>
@@ -53,29 +64,56 @@ describe Hbc::SystemCommand::Result, :cask do
       EOS
     }
 
-    context "when output contains garbage" do
+    context "when stdout contains garbage before XML" do
-      let(:input) {
+      let(:stdout) {
         <<~EOS
-          Hello there! I am in no way XML am I?!?!
+          #{garbage}
-
-            That's a little silly... you were expexting XML here!
-
-          What is a parser to do?
-
-          Hopefully <not> explode!
-
           #{plist}
         EOS
       }
 
-      it "ignores garbage before xml" do
+      it "ignores garbage" do
-        expect(subject.keys).to eq(["system-entities"])
         expect(subject["system-entities"].length).to eq(3)
       end
+
+      context "when verbose" do
+        before(:each) do
+          allow(ARGV).to receive(:verbose?).and_return(true)
+        end
+
+        it "warns about garbage" do
+          expect { subject }
+            .to output(a_string_containing(garbage)).to_stderr
+        end
+      end
     end
 
-    context "given a hdiutil output as input" do
+    context "when stdout contains garbage after XML" do
-      let(:input) { plist }
+      let(:stdout) {
+        <<~EOS
+          #{plist}
+          #{garbage}
+        EOS
+      }
+
+      it "ignores garbage" do
+        expect(subject["system-entities"].length).to eq(3)
+      end
+
+      context "when verbose" do
+        before(:each) do
+          allow(ARGV).to receive(:verbose?).and_return(true)
+        end
+
+        it "warns about garbage" do
+          expect { subject }
+            .to output(a_string_containing(garbage)).to_stderr
+        end
+      end
+    end
+
+    context "given a hdiutil stdout" do
+      let(:stdout) { plist }
 
       it "successfully parses it" do
         expect(subject.keys).to eq(["system-entities"])
@@ -85,11 +123,11 @@ describe Hbc::SystemCommand::Result, :cask do
       end
     end
 
-    context "given an empty input" do
+    context "when the stdout of the command is empty" do
-      let(:input) { "" }
+      let(:stdout) { "" }
 
-      it "raises an error" do
+      it "returns nil" do
-        expect { subject }.to raise_error(Hbc::CaskError, /Empty plist input/)
+        expect(subject).to be nil
       end
     end
   end

Library/Homebrew/test/spec_helper.rb

@@ -166,3 +166,4 @@ end
 
 RSpec::Matchers.define_negated_matcher :not_to_output, :output
 RSpec::Matchers.alias_matcher :have_failed, :be_failed
+RSpec::Matchers.alias_matcher :a_string_containing, :include