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

Author: ianfixes

Diff for: 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
 

Diff for: 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
@@ -62,6 +64,41 @@ 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
+
+    # 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)

Diff for: spec/arduino_backend_spec.rb

@@ -1,10 +1,32 @@
 require "spec_helper"
-require 'pathname'
+require "pathname"
+require "fileutils"
+require "tmpdir"
 
 def get_sketch(dir, file)
-  File.join(File.dirname(__FILE__), dir, file)
+  Pathname.new(__FILE__).parent + dir + file
 end
 
+# 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)
+    FileUtils.touch(config_file)
+    yield(config_dir, config_file)
+  end
+end
 
 RSpec.describe ArduinoCI::ArduinoBackend do
   next if skip_ruby_tests
@@ -27,6 +49,42 @@ def get_sketch(dir, file)
     end
   end
 
+  context "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))
+        expect(ret[:out].lines[0]).to include("Config file not found, using default values")
+      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("Using config file: #{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))
+        expect(ret[:out].lines[0]).to include("Using config file: #{config_file}")
+      end
+    end
+  end
+
   context "board_installed?" do
     it "Finds installed boards" do
       backend.install_boards("arduino:avr") # we used to assume this was installed... not the case for arduino-cli

Diff for: 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