Update Makefile log file key in diagnose report (#1022)

* Update Makefile log file key in diagnose report

As reported in #1018 the `mkmf.log` file path needs to be changed. It
can be found in the `extensions` directory. The path can be fetched with
the `extension_dir` method on the parsed gemspec.

This `extension_dir` method will only work when installed through Ruby
gems. If installed through a local path or Git repo it will need to use
the original behavior. If no file can be found, the `path` value is not
set.

I've updated the `gem_path` method to fetch the install path from the
gemspec as well. That way we have one source for this kind of
information.

Closes #1018

* Always return a path for the mkmf.log file

Even when no mkmf.log file is found, list the path that we expect it to
be in when installed. Might be easier to understand than seeing an empty
path for this file in the report.

.changesets/fix-makefile-log-path-reporting-in-diagnose-report.md

@@ -0,0 +1,6 @@
+---
+bump: "patch"
+type: "fix"
+---
+
+Fix the Makefile log path inclusion in the diagnose report. The diagnose tool didn't look in the correct gem extension directory for this log file.

lib/appsignal/cli/diagnose/paths.rb

@@ -19,7 +19,6 @@ def paths
             begin
               config = Appsignal.config
               log_file_path = config.log_file_path
-              makefile_log_path = File.join("ext", "mkmf.log")
               {
                 :package_install_path => {
                   :label => "AppSignal gem path",
@@ -37,9 +36,9 @@ def paths
                   :label => "Log directory",
                   :path => log_file_path ? File.dirname(log_file_path) : ""
                 },
-                makefile_log_path => {
+                "ext/mkmf.log" => {
                   :label => "Makefile install log",
-                  :path => File.join(gem_path, makefile_log_path)
+                  :path => makefile_install_log_path
                 },
                 "appsignal.log" => {
                   :label => "AppSignal log",
@@ -54,8 +53,11 @@ def paths
         def path_stat(path)
           {
             :path => path,
-            :exists => File.exist?(path)
+            :exists => false
           }.tap do |info|
+            next unless info[:path]
+
+            info[:exists] = File.exist?(path)
             next unless info[:exists]
 
             stat = File.stat(path)
@@ -84,9 +86,26 @@ def path_stat(path)
         end
 
         # Returns the AppSignal gem installation path. The root directory of
-        # this gem.
+        # this gem when installed.
         def gem_path
-          File.expand_path("../../../..", __dir__)
+          gemspec.full_gem_path
+        end
+
+        # Returns the AppSignal gem's Makefile log path, if it exists.
+        def makefile_install_log_path
+          possible_locations = [
+            # Installed gem location
+            File.join(gemspec.extension_dir, "mkmf.log"),
+            # Local development location
+            File.join(gem_path, "ext", "mkmf.log")
+          ]
+          possible_locations.find do |location|
+            File.exist?(location)
+          end || possible_locations.first
+        end
+
+        def gemspec
+          Gem.loaded_specs["appsignal"]
         end
       end
     end

spec/lib/appsignal/cli/diagnose_spec.rb

@@ -1357,6 +1357,7 @@ def expect_config_to_be_printed(config)
       shared_examples "diagnose file" do |shared_example_options|
         let(:parent_directory) { File.join(tmp_dir, "diagnose_files") }
         let(:file_path) { File.join(parent_directory, filename) }
+        let(:path_key) { filename }
         before { FileUtils.mkdir_p File.dirname(file_path) }
         after { FileUtils.rm_rf parent_directory }
 
@@ -1387,7 +1388,7 @@ def expect_config_to_be_printed(config)
           end
 
           it "transmits file data in report" do
-            expect(received_report["paths"][filename]).to match(
+            expect(received_report["paths"][path_key]).to match(
               "path" => file_path,
               "exists" => true,
               "type" => "file",
@@ -1418,7 +1419,7 @@ def expect_config_to_be_printed(config)
           end
 
           it "transmits file data in report" do
-            expect(received_report["paths"][filename]).to eq(
+            expect(received_report["paths"][path_key]).to eq(
               "path" => file_path,
               "exists" => false
             )
@@ -1439,20 +1440,22 @@ def expect_config_to_be_printed(config)
           end
 
           it "transmits file data in report" do
-            expect(received_report["paths"][filename]).to include(
+            expect(received_report["paths"][path_key]).to include(
               "read_error" => "Errno::ESPIPE: Illegal seek"
             )
           end
         end
       end
 
-      describe "mkmf.log" do
+      describe "ext/mkmf.log" do
         it_behaves_like "diagnose file" do
-          let(:filename) { File.join("ext", "mkmf.log") }
+          let(:filename) { "mkmf.log" }
+          let(:path_key) { "ext/mkmf.log" }
           before do
-            expect_any_instance_of(Appsignal::CLI::Diagnose::Paths).to receive(:gem_path)
+            expect_any_instance_of(Appsignal::CLI::Diagnose::Paths)
+              .to receive(:makefile_install_log_path)
               .at_least(:once)
-              .and_return(parent_directory)
+              .and_return(File.join(parent_directory, filename))
           end
         end