Explicit handling and future-proofing for arduino-cli#753 - config path

Author: ianfixes

CHANGELOG.md

@@ -23,6 +23,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Update .gitattributes so we have consistent line endings
 - Change 266 files from CRLF to LF.
 - Run tests on push as well as on a pull request so developers can see impact
+- `ArduinoBackend` now exposes `config_file_path` instead of `config_dir` so that we can be explicit about [strange behavior in `arduino-cli` that isn't going to change anytime soon](https://github.com/arduino/arduino-cli/issues/753)
 
 ### Deprecated
 

lib/arduino_ci/arduino_backend.rb

@@ -17,15 +17,17 @@ class ArduinoBackend
     # @return [String] the only allowable name for the arduino-cli config file.
     CONFIG_FILE_NAME = "arduino-cli.yaml".freeze
 
+    # Unfortunately we need error messaging around this quirk
+    # @return [String] The text to use for user apologies regarding the config file
+    CONFIG_FILE_APOLOGY = "Sorry this is weird, see https://github.com/arduino/arduino-cli/issues/753".freeze
+
     # the actual path to the executable on this platform
     # @return [Pathname]
     attr_accessor :binary_path
 
-    # If a custom config is deired (i.e. for testing), specify it here.
-    # Note https://github.com/arduino/arduino-cli/issues/753 : the --config-file option
-    # is really the director that contains the file
+    # The directory that contains the config file
     # @return [Pathname]
-    attr_accessor :config_dir
+    attr_reader :config_dir
 
     # @return [String] STDOUT of the most recently-run command
     attr_reader   :last_out
@@ -53,7 +55,7 @@ def _wrap_run(work_fn, *args, **kwargs)
       has_env = !args.empty? && args[0].instance_of?(Hash)
       env_vars = has_env ? args[0] : {}
       actual_args = has_env ? args[1..-1] : args  # need to shift over if we extracted args
-      custom_config = @config_dir.nil? ? [] : ["--config-file", @config_dir.to_s]
+      custom_config = @config_dir.nil? ? [] : ["--config-file", config_file_cli_param.to_s]
       full_args = [binary_path.to_s, "--format", "json"] + custom_config + actual_args
       full_cmd = env_vars.empty? ? full_args : [env_vars] + full_args
 
@@ -62,6 +64,52 @@ def _wrap_run(work_fn, *args, **kwargs)
       work_fn.call(*full_cmd, **kwargs)
     end
 
+    # The config file name to be passed on the command line
+    #
+    # Note https://github.com/arduino/arduino-cli/issues/753 : the --config-file option
+    # is really the directory that contains the file
+    #
+    # @return [Pathname]
+    def config_file_path
+      @config_dir + CONFIG_FILE_NAME
+    end
+
+    # The config file name to be passed on the command line
+    #
+    # Note https://github.com/arduino/arduino-cli/issues/753 : the --config-file option
+    # is really the directory that contains the file
+    #
+    # @param val [Pathname] The config file that will be used
+    # @return [Pathname]
+    def config_file_path=(rhs)
+      path_rhs = Pathname(rhs)
+      err_text = "Config file basename must be '#{CONFIG_FILE_NAME}'. #{CONFIG_FILE_APOLOGY}"
+      raise ArgumentError, err_text unless path_rhs.basename.to_s == CONFIG_FILE_NAME
+
+      @config_dir = path_rhs.dirname
+    end
+
+    # The config file to be used as a CLI param
+    #
+    # Apparently Linux wants the whole path, and OSX wants just the directory as of 0.29.0,
+    # it's all very annoying.  See unit tests.
+    #
+    # @return [Pathname] the path to use for a given OS
+    def config_file_cli_param
+      OS.linux? ? config_file_path : @config_dir
+    end
+
+    # Get an acceptable filename for use as a config file
+    #
+    # Note https://github.com/arduino/arduino-cli/issues/753 : the --config-file option
+    # is really the directory that contains the file
+    #
+    # @param dir [Pathname] the desired directory
+    # @return [Pathname]
+    def self.config_file_path_from_dir(dir)
+      Pathname(dir) + CONFIG_FILE_NAME
+    end
+
     # build and run the arduino command
     def run_and_output(*args, **kwargs)
       _wrap_run((proc { |*a, **k| Host.run_and_output(*a, **k) }), *args, **kwargs)

spec/arduino_backend_spec.rb

@@ -1,11 +1,9 @@
 require "spec_helper"
-require 'pathname'
 
 def get_sketch(dir, file)
-  File.join(File.dirname(__FILE__), dir, file)
+  Pathname.new(__FILE__).parent + dir + file
 end
 
-
 RSpec.describe ArduinoCI::ArduinoBackend do
   next if skip_ruby_tests
 

spec/arduino_installation_spec.rb

@@ -1,4 +1,37 @@
 require "spec_helper"
+require "pathname"
+require "tmpdir"
+require "os"
+
+# test function for below, to avoid long lines
+def bug_753_cmd(backend, config_file)
+  [
+    backend.binary_path.to_s,
+    "--config-file",
+    config_file.to_s,
+    "--verbose",
+    "config",
+    "dump"
+  ]
+end
+
+def with_tmp_file(desired_filename = nil)
+  Dir.mktmpdir do |tdir|
+    config_dir = Pathname(tdir)
+    config_file = config_dir + (desired_filename || ArduinoCI::ArduinoBackend::CONFIG_FILE_NAME)
+    File.open(config_file, "w") { |f| f.write("") }
+    yield(config_dir, config_file)
+  end
+end
+
+def config_success_msg(config_file)
+  config_file = config_file.gsub('/', '\\') if OS.windows?
+  "Using config file: #{config_file}"
+end
+
+def config_fail_msg
+  "Config file not found, using default values"
+end
 
 RSpec.describe ArduinoCI::ArduinoInstallation do
   next if skip_ruby_tests
@@ -35,4 +68,48 @@
     end
   end
 
+  context "installed version-specific quirks" do
+    backend = ArduinoCI::ArduinoInstallation.autolocate!
+
+    #  https://github.com/arduino/arduino-cli/issues/753
+
+    it "suffers from arduino-cli bug 753 - nonstandard filename" do
+      # foo.yml won't be accepted as a filename
+      with_tmp_file("foo.yml") do |config_dir, config_file|
+        expect(config_dir).to exist
+        expect(config_file).to exist
+        ret = ArduinoCI::Host.run_and_capture(*bug_753_cmd(backend, config_file))
+        if OS.linux?
+          expect(ret[:out].lines[0]).to include(config_success_msg(config_file))
+        else
+          expect(ret[:out].lines[0]).to include(config_fail_msg)
+        end
+      end
+    end
+
+    it "obeys arduino-cli bug 753 workaround" do
+      # the standard filename will work
+      with_tmp_file do |config_dir, config_file|
+        expect(config_dir).to exist
+        expect(config_file).to exist
+        ret = ArduinoCI::Host.run_and_capture(*bug_753_cmd(backend, config_file))
+        expect(ret[:out].lines[0]).to include(config_success_msg(config_file))
+      end
+    end
+
+    it "obeys arduino-cli bug 753" do
+      # the directory alone will work if there is a file with the right name
+      with_tmp_file do |config_dir, config_file|
+        expect(config_dir).to exist
+        expect(config_file).to exist
+        ret = ArduinoCI::Host.run_and_capture(*bug_753_cmd(backend, config_dir))
+        if OS.linux?
+          expect(ret[:out].lines[0]).to include(config_fail_msg)
+        else
+          expect(ret[:out].lines[0]).to include(config_success_msg(config_file))
+        end
+      end
+    end
+  end
+
 end

spec/fake_lib_dir.rb

@@ -11,9 +11,9 @@ class FakeLibDir
   def initialize
     # we will need to install some dummy libraries into a fake location, so do that on demand
     @config_dir = Pathname.new(Dir.pwd).realpath
-    @config_file = @config_dir + ArduinoCI::ArduinoBackend::CONFIG_FILE_NAME
+    @config_file = ArduinoCI::ArduinoBackend.config_file_path_from_dir(@config_dir)
     @backend = ArduinoCI::ArduinoInstallation.autolocate!
-    @backend.config_dir = @config_dir
+    @backend.config_file_path = @config_file
   end
 
   # designed to be called by rspec's "around" function