Print diagnose String values surrounded by quotes (#755)

This makes the Ruby integration print the diagnose output consistently with the Node.js integration: String values are surrounded by quotes when printed. I added a new `puts_format` helper method (please rename if you think of something better), to print the values like `puts_value` but print Strings with quotes around them. As a shortcut I call `#inspect` on the object to accomplish this. Other types of values should be printed like we expect as well. I split up some calls to `puts_and_save`. It's difficult to save it as the plain value, but print it as the "inspected" value. Now that I look at it several years later `puts_and_save` is very easy, but it does a little bit too much.
appsignal · Oct 20, 2021 · b40b3b4 · b40b3b4
1 parent c679a07
commit b40b3b4
Show file tree

Hide file tree

Showing 4 changed files with 70 additions and 42 deletions.
diff --git a/.changesets/format-printed-values.md b/.changesets/format-printed-values.md
@@ -0,0 +1,5 @@
+---
+bump: "patch"
+---
+
+Print String values in the diagnose report surrounded by quotes. Makes it more clear that it's a String value and not a label we print.
diff --git a/lib/appsignal/cli/diagnose.rb b/lib/appsignal/cli/diagnose.rb
@@ -171,11 +171,18 @@ def puts_and_save(key, label, value)
           puts_value label, value
         end
 
+        # Prints values as given. Does no formatting on the value
         def puts_value(label, value, options = {})
           options[:level] ||= 1
           puts "#{"  " * options[:level]}#{label}: #{value}"
         end
 
+        # Print values as inspected.
+        # Surrounds Strings in quotes.
+        def puts_format(label, value, options = {})
+          puts_value label, value.inspect, options
+        end
+
         def configure_appsignal(options)
           current_path = Dir.pwd
           initial_config = {}
@@ -327,9 +334,18 @@ def library_information
           data_section :library do
             save :language, "ruby"
             puts_value "Language", "Ruby"
-            puts_and_save :package_version, "Gem version", Appsignal::VERSION
-            puts_and_save :agent_version, "Agent version", Appsignal::Extension.agent_version
-            puts_and_save :extension_loaded, "Extension loaded", Appsignal.extension_loaded
+
+            package_version = Appsignal::VERSION
+            save :package_version, package_version
+            puts_format "Gem version", package_version
+
+            agent_version = Appsignal::Extension.agent_version
+            save :agent_version, agent_version
+            puts_format "Agent version", agent_version
+
+            extension_loaded = Appsignal.extension_loaded
+            save :extension_loaded, extension_loaded
+            puts_format "Extension loaded", extension_loaded
           end
         end
 
@@ -381,29 +397,29 @@ def print_installation_result_report(report)
         def print_installation_language_report(report)
           report = report.fetch("language", {})
           puts "  Language details"
-          puts "    Implementation: #{report["implementation"]}"
-          puts "    Ruby version: #{report["version"]}"
+          puts_format "Implementation", report["implementation"], :level => 2
+          puts_format "Ruby version", report["version"], :level => 2
         end
 
         def print_installation_download_report(report)
           report = report.fetch("download", {})
           puts "  Download details"
-          puts "    Download URL: #{report["download_url"]}"
-          puts "    Checksum: #{report["checksum"]}"
+          puts_format "Download URL", report["download_url"], :level => 2
+          puts_format "Checksum", report["checksum"], :level => 2
         end
 
         def print_installation_build_report(report)
           report = report.fetch("build", {})
           puts "  Build details"
-          puts "    Install time: #{report["time"]}"
-          puts "    Architecture: #{report["architecture"]}"
-          puts "    Target: #{report["target"]}"
-          puts "    Musl override: #{report["musl_override"]}"
-          puts "    Linux ARM override: #{report["linux_arm_override"]}"
-          puts "    Library type: #{report["library_type"]}"
-          puts "    Source: #{report["source"]}" if report["source"] != "remote"
-          puts "    Dependencies: #{report["dependencies"]}"
-          puts "    Flags: #{report["flags"]}"
+          puts_format "Install time", report["time"].to_s, :level => 2
+          puts_format "Architecture", report["architecture"], :level => 2
+          puts_format "Target", report["target"], :level => 2
+          puts_format "Musl override", report["musl_override"], :level => 2
+          puts_format "Linux ARM override", report["linux_arm_override"], :level => 2
+          puts_format "Library type", report["library_type"], :level => 2
+          puts_format "Source", report["source"], :level => 2 if report["source"] != "remote"
+          puts_format "Dependencies", report["dependencies"], :level => 2
+          puts_format "Flags", report["flags"], :level => 2
         end
 
         def print_installation_host_report(report)
@@ -417,24 +433,31 @@ def host_information
           rbconfig = RbConfig::CONFIG
           puts "Host information"
           data_section :host do
-            puts_and_save :architecture, "Architecture", Appsignal::System.agent_architecture
+            agent_architecture = Appsignal::System.agent_architecture
+            save :architecture, agent_architecture
+            puts_format "Architecture", agent_architecture
 
-            os_label = os = rbconfig["host_os"]
-            os_label = "#{os} (Microsoft Windows is not supported.)" if Gem.win_platform?
+            os = rbconfig["host_os"]
+            os_label = os.inspect
+            os_label = "#{os_label} (Microsoft Windows is not supported.)" if Gem.win_platform?
             save :os, os
             puts_value "Operating System", os_label
 
-            puts_and_save :language_version, "Ruby version",
-              "#{rbconfig["ruby_version"]}-p#{rbconfig["PATCHLEVEL"]}"
+            language_version = "#{rbconfig["ruby_version"]}-p#{rbconfig["PATCHLEVEL"]}"
+            save :language_version, language_version
+            puts_format "Ruby version", language_version
+
+            heroku = Appsignal::System.heroku?
+            save :heroku, heroku
+            puts_format "Heroku", true if Appsignal::System.heroku?
 
-            puts_value "Heroku", "true" if Appsignal::System.heroku?
-            save :heroku, Appsignal::System.heroku?
+            root = Process.uid.zero?
+            save :root, root
+            puts_value "Root user", root ? "true (not recommended)" : "false"
 
-            save :root, Process.uid.zero?
-            puts_value "Root user",
-              Process.uid.zero? ? "true (not recommended)" : "false"
-            puts_and_save :running_in_container, "Running in container",
-              Appsignal::Extension.running_in_container?
+            running_in_container = Appsignal::Extension.running_in_container?
+            save :running_in_container, running_in_container
+            puts_format "Running in container", running_in_container
           end
         end
 

diff --git a/spec/integration/diagnose b/spec/integration/diagnose
diff --git a/spec/lib/appsignal/cli/diagnose_spec.rb b/spec/lib/appsignal/cli/diagnose_spec.rb
@@ -191,8 +191,8 @@ def dont_accept_prompt_to_send_diagnostics_report
 
       it "outputs version numbers" do
         expect(output).to include \
-          "Gem version: #{Appsignal::VERSION}",
-          "Agent version: #{Appsignal::Extension.agent_version}"
+          "Gem version: \"#{Appsignal::VERSION}\"",
+          "Agent version: \"#{Appsignal::Extension.agent_version}\""
       end
 
       it "transmits version numbers in report" do
@@ -294,18 +294,18 @@ def dont_accept_prompt_to_send_diagnostics_report
           "Installation result",
           "  Status: success",
           "Language details",
-          "  Implementation: #{jruby ? "jruby" : "ruby"}",
-          "  Ruby version: #{"#{rbconfig["ruby_version"]}-p#{rbconfig["PATCHLEVEL"]}"}",
+          "  Implementation: \"#{jruby ? "jruby" : "ruby"}\"",
+          "  Ruby version: \"#{"#{rbconfig["ruby_version"]}-p#{rbconfig["PATCHLEVEL"]}"}\"",
           "Download details",
-          "  Download URL: https://",
-          "  Checksum: verified",
+          "  Download URL: \"https://",
+          "  Checksum: \"verified\"",
           "Build details",
-          "  Install time: 20",
-          "  Architecture: #{Appsignal::System.agent_architecture}",
-          "  Target: #{Appsignal::System.agent_platform}",
+          "  Install time: \"20",
+          "  Architecture: \"#{Appsignal::System.agent_architecture}\"",
+          "  Target: \"#{Appsignal::System.agent_platform}\"",
           "  Musl override: false",
           "  Linux ARM override: false",
-          "  Library type: #{jruby ? "dynamic" : "static"}",
+          "  Library type: \"#{jruby ? "dynamic" : "static"}\"",
           "  Dependencies: {",
           "  Flags: {",
           "Host details",
@@ -610,9 +610,9 @@ def dont_accept_prompt_to_send_diagnostics_report
         run
         expect(output).to include \
           "Host information",
-          "Architecture: #{rbconfig["host_cpu"]}",
-          "Operating System: #{rbconfig["host_os"]}",
-          "Ruby version: #{language_version}"
+          "Architecture: \"#{rbconfig["host_cpu"]}\"",
+          "Operating System: \"#{rbconfig["host_os"]}\"",
+          "Ruby version: \"#{language_version}\""
       end
 
       context "when on Microsoft Windows" do
+1 −0		Gemfile
+6 −0		Gemfile.lock
+17 −17		spec/diagnose_spec.rb
+2 −2		spec/support/output_helper.rb
+11 −7		spec/support/runner.rb