From ab47c85ef327c7546a73acc7291b8ed7a6502b75 Mon Sep 17 00:00:00 2001 From: Ian Katz Date: Wed, 10 Jan 2018 12:17:25 -0500 Subject: [PATCH 01/22] split some files and add installer functionality --- CHANGELOG.md | 2 + lib/arduino_ci.rb | 118 +----------------- lib/arduino_ci/arduino_cmd.rb | 34 +++++ lib/arduino_ci/arduino_installation.rb | 77 ++++++++++++ lib/arduino_ci/display_manager.rb | 74 +++++++++++ spec/arduino_cmd_spec.rb | 9 ++ spec/arduino_installation_spec.rb | 25 ++++ ...o_exec_spec.rb => display_manager_spec.rb} | 15 +-- 8 files changed, 226 insertions(+), 128 deletions(-) create mode 100644 lib/arduino_ci/arduino_cmd.rb create mode 100644 lib/arduino_ci/arduino_installation.rb create mode 100644 lib/arduino_ci/display_manager.rb create mode 100644 spec/arduino_cmd_spec.rb create mode 100644 spec/arduino_installation_spec.rb rename spec/{arduino_exec_spec.rb => display_manager_spec.rb} (51%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 72f7c402..dc516990 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] ### Added +- `ArduinoInstallation` class for managing lib / executable paths +- `DisplayManager` class for managing Xvfb instance if needed ### Changed diff --git a/lib/arduino_ci.rb b/lib/arduino_ci.rb index 7edf9596..2f871c67 100644 --- a/lib/arduino_ci.rb +++ b/lib/arduino_ci.rb @@ -1,124 +1,8 @@ require "arduino_ci/version" - -require 'singleton' - -# Cross-platform way of finding an executable in the $PATH. -# via https://stackoverflow.com/a/5471032/2063546 -# which('ruby') #=> /usr/bin/ruby -def which(cmd) - exts = ENV['PATHEXT'] ? ENV['PATHEXT'].split(';') : [''] - ENV['PATH'].split(File::PATH_SEPARATOR).each do |path| - exts.each do |ext| - exe = File.join(path, "#{cmd}#{ext}") - return exe if File.executable?(exe) && !File.directory?(exe) - end - end - nil -end +require "arduino_ci/arduino_cmd" # ArduinoCI contains classes for automated testing of Arduino code on the command line # @author Ian Katz module ArduinoCI - # Wrap the Arduino executable. This requires, in some cases, a faked display. - class ArduinoCmd - - # create as many ArduinoCmds as you like, but we need one and only one display manager - class DisplayMgr - include Singleton - attr_reader :enabled - - def initialize - @existing = existing_display? - @enabled = false - @pid = nil - end - - # attempt to determine if the machine is running a graphical display (i.e. not Travis) - def existing_display? - return true if RUBY_PLATFORM.include? "darwin" - return true if ENV["DISPLAY"].nil? - return true if ENV["DISPLAY"].include? ":" - false - end - - # enable a virtual display - def enable - return @enabled = true if @existing # silent no-op if built in display - return unless @pid.nil? - - @enabled = true - @pid = fork do - puts "Forking Xvfb" - system("Xvfb", ":1", "-ac", "-screen", "0", "1280x1024x16") - puts "Xvfb unexpectedly quit!" - end - sleep(3) # TODO: test a connection to the X server? - end - - # disable the virtual display - def disable - return @enabled = false if @existing # silent no-op if built in display - return if @pid.nil? - - begin - Process.kill 9, @pid - ensure - Process.wait @pid - @pid = nil - end - puts "Xvfb killed" - end - - # Enable a virtual display for the duration of the given block - def with_display - enable - begin - yield environment - ensure - disable - end - end - - def environment - return nil unless @existing || @enabled - return {} if @existing - { DISPLAY => ":1.0" } - end - - # On finalize, ensure child process is ended - def self.finalize - disable - end - end - - class << self - protected :new - - # attempt to find a workable Arduino executable across platforms - def guess_executable_location - osx_place = "/Applications/Arduino.app/Contents/MacOS/Arduino" - places = { - "arduino" => !which("arduino").nil?, - osx_place => (File.exist? osx_place), - } - places.each { |k, v| return k if v } - nil - end - - def autolocate - ret = new - ret.path = guess_executable_location - ret - end - end - - attr_accessor :path - - def initialize - @display_mgr = DisplayMgr::instance - end - - end - end diff --git a/lib/arduino_ci/arduino_cmd.rb b/lib/arduino_ci/arduino_cmd.rb new file mode 100644 index 00000000..13ffc1ed --- /dev/null +++ b/lib/arduino_ci/arduino_cmd.rb @@ -0,0 +1,34 @@ +require 'arduino_ci/display_manager' +require 'arduino_ci/arduino_installation' + +module ArduinoCI + + # Wrap the Arduino executable. This requires, in some cases, a faked display. + class ArduinoCmd + + class << self + protected :new + + def autolocate + new(ArduinoInstallation.autolocate) + end + + def autolocate! + new(ArduinoInstallation.autolocate!) + end + + end + + attr_accessor :installation + + def initialize(installation) + @display_mgr = DisplayManager::instance + @installation = installation + end + + def board_installed?(board) + system(@installation, "--board", board) + end + + end +end diff --git a/lib/arduino_ci/arduino_installation.rb b/lib/arduino_ci/arduino_installation.rb new file mode 100644 index 00000000..231c3ee8 --- /dev/null +++ b/lib/arduino_ci/arduino_installation.rb @@ -0,0 +1,77 @@ +# Cross-platform way of finding an executable in the $PATH. +# via https://stackoverflow.com/a/5471032/2063546 +# which('ruby') #=> /usr/bin/ruby +def which(cmd) + exts = ENV['PATHEXT'] ? ENV['PATHEXT'].split(';') : [''] + ENV['PATH'].split(File::PATH_SEPARATOR).each do |path| + exts.each do |ext| + exe = File.join(path, "#{cmd}#{ext}") + return exe if File.executable?(exe) && !File.directory?(exe) + end + end + nil +end + +module ArduinoCI + + # Manage the OS-specific install location of Arduino + class ArduinoInstallation + attr_accessor :cmd_path + attr_accessor :lib_dir + + class << self + def force_install_location + File.join(ENV['HOME'], 'arduino_ci_ide') + end + + # attempt to find a workable Arduino executable across platforms + def autolocate + ret = new + + osx_place = "/Applications/Arduino.app/Contents/MacOS/Arduino" + if File.exist? osx_place + ret.cmd_path = File.join(osx_place, "arduino") + ret.lib_dir = File.join(osx_place, "Libraries") + return ret + end + + posix_place = which("arduino") + unless posix_place.nil? + ret.cmd_path = posix_place + ret.lib_dir = File.join(ENV['HOME'], "Sketchbook") # assume linux + # https://learn.adafruit.com/adafruit-all-about-arduino-libraries-install-use/how-to-install-a-library + return ret + end + + if File.exist? force_install_location + ret.cmd_path = File.join(force_install_location, "arduino") + ret.lib_dir = File.join(force_install_location, "libraries") + # TODO: "libraries" is what's in the adafruit install.sh script + return ret + end + + ret + end + + # Attempt to find a workable Arduino executable across platforms, and install it if we don't + def autolocate! + candidate = autolocate + return candidate unless candidate.cmd_path.nil? + # force the install + + if force_install + candidate.cmd_path = File.join(force_install_location, "arduino") + candidate.lib_dir = File.join(force_install_location, "libraries") + end + candidate + end + + def force_install + system("wget", "https://downloads.arduino.cc/arduino-1.6.5-linux64.tar.xz") + system("tar", "xf", "arduino-1.6.5-linux64.tar.xz") + system("mv", "arduino-1.6.5", force_install_location) + end + + end + end +end diff --git a/lib/arduino_ci/display_manager.rb b/lib/arduino_ci/display_manager.rb new file mode 100644 index 00000000..5e540053 --- /dev/null +++ b/lib/arduino_ci/display_manager.rb @@ -0,0 +1,74 @@ +require 'singleton' + +module ArduinoCI + + # When arduino commands run, they need a graphical display. + # This class handles the setup of that display, if needed. + class DisplayManager + include Singleton + attr_reader :enabled + + def initialize + @existing = existing_display? + @enabled = false + @pid = nil + end + + # attempt to determine if the machine is running a graphical display (i.e. not Travis) + def existing_display? + return true if RUBY_PLATFORM.include? "darwin" + return true if ENV["DISPLAY"].nil? + return true if ENV["DISPLAY"].include? ":" + false + end + + # enable a virtual display + def enable + return @enabled = true if @existing # silent no-op if built in display + return unless @pid.nil? + + @enabled = true + @pid = fork do + puts "Forking Xvfb" + system("Xvfb", ":1", "-ac", "-screen", "0", "1280x1024x16") + puts "Xvfb unexpectedly quit!" + end + sleep(3) # TODO: test a connection to the X server? + end + + # disable the virtual display + def disable + return @enabled = false if @existing # silent no-op if built in display + return if @pid.nil? + + begin + Process.kill 9, @pid + ensure + Process.wait @pid + @pid = nil + end + puts "Xvfb killed" + end + + # Enable a virtual display for the duration of the given block + def with_display + enable + begin + yield environment + ensure + disable + end + end + + def environment + return nil unless @existing || @enabled + return {} if @existing + { DISPLAY => ":1.0" } + end + + # On finalize, ensure child process is ended + def self.finalize + disable + end + end +end diff --git a/spec/arduino_cmd_spec.rb b/spec/arduino_cmd_spec.rb new file mode 100644 index 00000000..5d0b2598 --- /dev/null +++ b/spec/arduino_cmd_spec.rb @@ -0,0 +1,9 @@ +require "spec_helper" + +RSpec.describe ArduinoCI::ArduinoCmd do + it "Finds the Arduino executable" do + arduino_cmd = ArduinoCI::ArduinoCmd.autolocate + # expect(arduino_cmd.path).not_to be nil + end +end + diff --git a/spec/arduino_installation_spec.rb b/spec/arduino_installation_spec.rb new file mode 100644 index 00000000..c0a81278 --- /dev/null +++ b/spec/arduino_installation_spec.rb @@ -0,0 +1,25 @@ +require "spec_helper" + +RSpec.describe ArduinoCI::ArduinoInstallation do + context "force_install_location" do + it "is resolvable" do + expect(ArduinoCI::ArduinoInstallation.force_install_location).not_to be nil + end + end + + context "autolocate" do + it "doesn't fail" do + ArduinoCI::ArduinoInstallation.autolocate + end + end + + context "autolocate!" do + it "doesn't fail" do + installation = ArduinoCI::ArduinoInstallation.autolocate! + expect(installation.cmd_path).not_to be nil + expect(installation.lib_dir).not_to be nil + end + end + +end + diff --git a/spec/arduino_exec_spec.rb b/spec/display_manager_spec.rb similarity index 51% rename from spec/arduino_exec_spec.rb rename to spec/display_manager_spec.rb index 19d2147a..3abbab97 100644 --- a/spec/arduino_exec_spec.rb +++ b/spec/display_manager_spec.rb @@ -1,27 +1,20 @@ require "spec_helper" -RSpec.describe ArduinoCI::ArduinoCmd do - it "Finds the Arduino executable" do - arduino_cmd = ArduinoCI::ArduinoCmd.autolocate - # expect(arduino_cmd.path).not_to be nil - end -end - -RSpec.describe ArduinoCI::ArduinoCmd::DisplayMgr do +RSpec.describe ArduinoCI::DisplayManager do context "singleton ::instance" do it "produces an instance" do - expect(ArduinoCI::ArduinoCmd::DisplayMgr::instance).not_to be_nil + expect(ArduinoCI::DisplayManager::instance).not_to be_nil end end context "with_display" do it "Properly enables and disables" do - manager = ArduinoCI::ArduinoCmd::DisplayMgr::instance + manager = ArduinoCI::DisplayManager::instance expect(manager.enabled).to be false manager.with_display do |environment| expect(manager.enabled).to be true expect(environment.class).to eq(Hash) - also_manager = ArduinoCI::ArduinoCmd::DisplayMgr::instance + also_manager = ArduinoCI::DisplayManager::instance expect(also_manager.enabled).to be true end expect(manager.enabled).to be false From d736446b3b0bfdc253a1438c255c83e427bac537 Mon Sep 17 00:00:00 2001 From: Ian Katz Date: Wed, 10 Jan 2018 12:25:50 -0500 Subject: [PATCH 02/22] don't disable the display if previously enabled outside with_display --- CHANGELOG.md | 1 + lib/arduino_ci/display_manager.rb | 5 +++-- spec/display_manager_spec.rb | 18 ++++++++++++++++-- 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dc516990..57ddf4fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - `DisplayManager` class for managing Xvfb instance if needed ### Changed +- `DisplayManger.with_display` doesn't `disable` if the display was enabled prior to starting the block ### Deprecated diff --git a/lib/arduino_ci/display_manager.rb b/lib/arduino_ci/display_manager.rb index 5e540053..f9e1df83 100644 --- a/lib/arduino_ci/display_manager.rb +++ b/lib/arduino_ci/display_manager.rb @@ -52,11 +52,12 @@ def disable # Enable a virtual display for the duration of the given block def with_display - enable + was_enabled = @enabled + enable unless was_enabled begin yield environment ensure - disable + disable unless was_enabled end end diff --git a/spec/display_manager_spec.rb b/spec/display_manager_spec.rb index 3abbab97..60b25456 100644 --- a/spec/display_manager_spec.rb +++ b/spec/display_manager_spec.rb @@ -8,8 +8,10 @@ end context "with_display" do - it "Properly enables and disables" do - manager = ArduinoCI::DisplayManager::instance + manager = ArduinoCI::DisplayManager::instance + manager.disable + + it "Properly enables and disables when not previously enabled" do expect(manager.enabled).to be false manager.with_display do |environment| expect(manager.enabled).to be true @@ -19,5 +21,17 @@ end expect(manager.enabled).to be false end + + it "Properly enables and disables when previously enabled" do + manager.enable + expect(manager.enabled).to be true + manager.with_display do |environment| + expect(manager.enabled).to be true + expect(environment.class).to eq(Hash) + also_manager = ArduinoCI::DisplayManager::instance + expect(also_manager.enabled).to be true + end + expect(manager.enabled).to be true + end end end From effc5109e44ebbb5b2bd7fcbc0a6b5f90c3ecb86 Mon Sep 17 00:00:00 2001 From: Ian Katz Date: Wed, 10 Jan 2018 12:41:18 -0500 Subject: [PATCH 03/22] fix path for OSX --- lib/arduino_ci/arduino_installation.rb | 4 ++-- spec/arduino_cmd_spec.rb | 14 +++++++++++--- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/lib/arduino_ci/arduino_installation.rb b/lib/arduino_ci/arduino_installation.rb index 231c3ee8..f72ff402 100644 --- a/lib/arduino_ci/arduino_installation.rb +++ b/lib/arduino_ci/arduino_installation.rb @@ -28,9 +28,9 @@ def force_install_location def autolocate ret = new - osx_place = "/Applications/Arduino.app/Contents/MacOS/Arduino" + osx_place = "/Applications/Arduino.app/Contents/MacOS" if File.exist? osx_place - ret.cmd_path = File.join(osx_place, "arduino") + ret.cmd_path = File.join(osx_place, "Arduino") ret.lib_dir = File.join(osx_place, "Libraries") return ret end diff --git a/spec/arduino_cmd_spec.rb b/spec/arduino_cmd_spec.rb index 5d0b2598..ad461de7 100644 --- a/spec/arduino_cmd_spec.rb +++ b/spec/arduino_cmd_spec.rb @@ -1,9 +1,17 @@ require "spec_helper" RSpec.describe ArduinoCI::ArduinoCmd do - it "Finds the Arduino executable" do - arduino_cmd = ArduinoCI::ArduinoCmd.autolocate - # expect(arduino_cmd.path).not_to be nil + context "autolocate" do + it "Finds the Arduino executable" do + arduino_cmd = ArduinoCI::ArduinoCmd.autolocate + end + end + + context "autolocate!" do + it "Finds the Arduino executable" do + arduino_cmd = ArduinoCI::ArduinoCmd.autolocate! + expect(arduino_cmd.installation.cmd_path).not_to be nil + end end end From 64e272dae73fa1d62856f9677113ea504dea2345 Mon Sep 17 00:00:00 2001 From: Ian Katz Date: Wed, 10 Jan 2018 12:42:20 -0500 Subject: [PATCH 04/22] add test for the board-is-installed function --- CHANGELOG.md | 1 + spec/arduino_cmd_spec.rb | 13 ++++++++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 57ddf4fb..ccc89a72 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Added - `ArduinoInstallation` class for managing lib / executable paths - `DisplayManager` class for managing Xvfb instance if needed +- `ArduinoCmd` can report on whether a board is installed ### Changed - `DisplayManger.with_display` doesn't `disable` if the display was enabled prior to starting the block diff --git a/spec/arduino_cmd_spec.rb b/spec/arduino_cmd_spec.rb index ad461de7..ac44d081 100644 --- a/spec/arduino_cmd_spec.rb +++ b/spec/arduino_cmd_spec.rb @@ -13,5 +13,16 @@ expect(arduino_cmd.installation.cmd_path).not_to be nil end end -end + context "board_installed?" do + arduino_cmd = ArduinoCI::ArduinoCmd.autolocate! + ArduinoCI::DisplayManager::instance.enable + it "Finds installed boards" do + expect(arduino_cmd.board_installed? "arduino:avr:uno").to be true + end + + it "Doesn't find bogus boards" do + expect(arduino_cmd.board_installed? "eggs:milk:wheat").to be false + end + end +end From 341034de023310c73eac2f20a13330b55199a171 Mon Sep 17 00:00:00 2001 From: Ian Katz Date: Wed, 10 Jan 2018 15:55:21 -0500 Subject: [PATCH 05/22] add CI script to gem --- .travis.yml | 1 + arduino_ci.gemspec | 7 +++++-- exe/ci_system_check.rb | 23 +++++++++++++++++++++++ lib/arduino_ci/arduino_cmd.rb | 8 +++++++- 4 files changed, 36 insertions(+), 3 deletions(-) create mode 100755 exe/ci_system_check.rb diff --git a/.travis.yml b/.travis.yml index 08dae787..9a3c487f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -13,3 +13,4 @@ script: - bundle exec rubocop --version - bundle exec rubocop -D . - bundle exec rspec + - bundle exec ci_system_check.rb diff --git a/arduino_ci.gemspec b/arduino_ci.gemspec index 1ba46750..b25f8ba6 100644 --- a/arduino_ci.gemspec +++ b/arduino_ci.gemspec @@ -14,9 +14,12 @@ Gem::Specification.new do |spec| spec.description = spec.description spec.homepage = "http://github.com/ifreecarve/arduino_ci" - spec.files = ['README.md', '.yardopts'] + Dir['lib/**/*.*'].reject { |f| f.match(%r{^(test|spec|features)/}) } - spec.bindir = "exe" + rejection_regex = %r{^(test|spec|features)/} + libfiles = Dir['lib/**/*.*'].reject { |f| f.match(rejection_regex) } + binfiles = Dir[File.join(spec.bindir, '/**/*.*')].reject { |f| f.match(rejection_regex) } + spec.files = ['README.md', '.yardopts'] + libfiles + binfiles + spec.executables = spec.files.grep(%r{^exe/}) { |f| File.basename(f) } spec.require_paths = ["lib"] diff --git a/exe/ci_system_check.rb b/exe/ci_system_check.rb new file mode 100755 index 00000000..bbd104ad --- /dev/null +++ b/exe/ci_system_check.rb @@ -0,0 +1,23 @@ +require 'arduino_ci' + +puts "Enabling display with display manager" +ArduinoCI::DisplayManager::instance.enable + +puts "Autlocating Arduino command" +arduino_cmd = ArduinoCI::ArduinoCmd.autolocate! + +board_tests = { + "arduino:avr:uno" => true, + "eggs:milk:wheat" => false, +} + +got_problem = false +board_tests.each do |k, v| + puts "I expect arduino_cmd.board_installed?(#{k}) to be #{v}" + result = arduino_cmd.board_installed?(k) + puts " board_installed?(#{k}) returns #{result}. expected #{v}" + got_problem = true if v != result +end + +abort if got_problem +exit(0) diff --git a/lib/arduino_ci/arduino_cmd.rb b/lib/arduino_ci/arduino_cmd.rb index 13ffc1ed..7c8ef168 100644 --- a/lib/arduino_ci/arduino_cmd.rb +++ b/lib/arduino_ci/arduino_cmd.rb @@ -26,8 +26,14 @@ def initialize(installation) @installation = installation end + def run(*args) + full_args = [@installation.cmd_path] + args + puts "Running $ #{full_args.join(' ')}" + system(*full_args) + end + def board_installed?(board) - system(@installation, "--board", board) + run("--board", board) end end From 2119ebc217159a8bb1b53e820b8306411a51d517 Mon Sep 17 00:00:00 2001 From: Ian Katz Date: Thu, 11 Jan 2018 07:00:50 -0500 Subject: [PATCH 06/22] wrap 'which' method in its own class and add tests --- lib/arduino_ci/arduino_installation.rb | 16 ++-------------- lib/arduino_ci/display_manager.rb | 1 + lib/arduino_ci/host.rb | 19 +++++++++++++++++++ spec/arduino_ci_spec.rb | 17 +++++++++++++++-- 4 files changed, 37 insertions(+), 16 deletions(-) create mode 100644 lib/arduino_ci/host.rb diff --git a/lib/arduino_ci/arduino_installation.rb b/lib/arduino_ci/arduino_installation.rb index f72ff402..10ef76fd 100644 --- a/lib/arduino_ci/arduino_installation.rb +++ b/lib/arduino_ci/arduino_installation.rb @@ -1,16 +1,4 @@ -# Cross-platform way of finding an executable in the $PATH. -# via https://stackoverflow.com/a/5471032/2063546 -# which('ruby') #=> /usr/bin/ruby -def which(cmd) - exts = ENV['PATHEXT'] ? ENV['PATHEXT'].split(';') : [''] - ENV['PATH'].split(File::PATH_SEPARATOR).each do |path| - exts.each do |ext| - exe = File.join(path, "#{cmd}#{ext}") - return exe if File.executable?(exe) && !File.directory?(exe) - end - end - nil -end +require "arduino_ci/host" module ArduinoCI @@ -35,7 +23,7 @@ def autolocate return ret end - posix_place = which("arduino") + posix_place = Host.which("arduino") unless posix_place.nil? ret.cmd_path = posix_place ret.lib_dir = File.join(ENV['HOME'], "Sketchbook") # assume linux diff --git a/lib/arduino_ci/display_manager.rb b/lib/arduino_ci/display_manager.rb index f9e1df83..1965e558 100644 --- a/lib/arduino_ci/display_manager.rb +++ b/lib/arduino_ci/display_manager.rb @@ -1,3 +1,4 @@ +require 'arduino_ci/host' require 'singleton' module ArduinoCI diff --git a/lib/arduino_ci/host.rb b/lib/arduino_ci/host.rb new file mode 100644 index 00000000..aeed2c2f --- /dev/null +++ b/lib/arduino_ci/host.rb @@ -0,0 +1,19 @@ +module ArduinoCI + + # Tools for interacting with the host machine + class Host + # Cross-platform way of finding an executable in the $PATH. + # via https://stackoverflow.com/a/5471032/2063546 + # which('ruby') #=> /usr/bin/ruby + def self.which(cmd) + exts = ENV['PATHEXT'] ? ENV['PATHEXT'].split(';') : [''] + ENV['PATH'].split(File::PATH_SEPARATOR).each do |path| + exts.each do |ext| + exe = File.join(path, "#{cmd}#{ext}") + return exe if File.executable?(exe) && !File.directory?(exe) + end + end + nil + end + end +end diff --git a/spec/arduino_ci_spec.rb b/spec/arduino_ci_spec.rb index 0f3d9c57..e63fca26 100644 --- a/spec/arduino_ci_spec.rb +++ b/spec/arduino_ci_spec.rb @@ -1,7 +1,20 @@ require "spec_helper" RSpec.describe ArduinoCI do - it "has a version number" do - expect(ArduinoCI::VERSION).not_to be nil + context "gem" do + it "has a version number" do + expect(ArduinoCI::VERSION).not_to be nil + end end end + +RSpec.describe ArduinoCI::Host do + context "which" do + it "can find things with which" do + ruby_path = ArduinoCI::Host.which("ruby") + expect(ruby_path).not_to be nil + expect(ruby_path.include? "ruby").to be true + end + end + +end From a247e2d58ad4f4c67b2ad5754d8afd419f4d1e7b Mon Sep 17 00:00:00 2001 From: Ian Katz Date: Thu, 11 Jan 2018 07:01:28 -0500 Subject: [PATCH 07/22] be more verbose about skipping display manager enable --- lib/arduino_ci/display_manager.rb | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/arduino_ci/display_manager.rb b/lib/arduino_ci/display_manager.rb index 1965e558..7a4ad388 100644 --- a/lib/arduino_ci/display_manager.rb +++ b/lib/arduino_ci/display_manager.rb @@ -25,8 +25,13 @@ def existing_display? # enable a virtual display def enable - return @enabled = true if @existing # silent no-op if built in display - return unless @pid.nil? + if @existing + puts "DisplayManager: no-op for what appears to be an existing display" + @enabled = true + return + end + + return unless @pid.nil? # TODO: disable first? @enabled = true @pid = fork do From a1154665cb65886e72cf2def3df433a63b31d6bd Mon Sep 17 00:00:00 2001 From: Ian Katz Date: Thu, 11 Jan 2018 07:06:54 -0500 Subject: [PATCH 08/22] fix logic in existing_display? --- lib/arduino_ci/display_manager.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/arduino_ci/display_manager.rb b/lib/arduino_ci/display_manager.rb index 7a4ad388..bb223964 100644 --- a/lib/arduino_ci/display_manager.rb +++ b/lib/arduino_ci/display_manager.rb @@ -17,16 +17,16 @@ def initialize # attempt to determine if the machine is running a graphical display (i.e. not Travis) def existing_display? - return true if RUBY_PLATFORM.include? "darwin" - return true if ENV["DISPLAY"].nil? - return true if ENV["DISPLAY"].include? ":" + return true if RUBY_PLATFORM.include? "darwin" + return false if ENV["DISPLAY"].nil? + return true if ENV["DISPLAY"].include? ":" false end # enable a virtual display def enable if @existing - puts "DisplayManager: no-op for what appears to be an existing display" + puts "DisplayManager enable: no-op for what appears to be an existing display" @enabled = true return end From 5782cff73298f16ae836ae43a3222c04cbd59aec Mon Sep 17 00:00:00 2001 From: Ian Katz Date: Thu, 11 Jan 2018 07:57:27 -0500 Subject: [PATCH 09/22] use IO.popen instead of forking a fork --- lib/arduino_ci/display_manager.rb | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/lib/arduino_ci/display_manager.rb b/lib/arduino_ci/display_manager.rb index bb223964..6eedb4ee 100644 --- a/lib/arduino_ci/display_manager.rb +++ b/lib/arduino_ci/display_manager.rb @@ -1,5 +1,6 @@ require 'arduino_ci/host' require 'singleton' +require 'timeout' module ArduinoCI @@ -34,26 +35,36 @@ def enable return unless @pid.nil? # TODO: disable first? @enabled = true - @pid = fork do - puts "Forking Xvfb" - system("Xvfb", ":1", "-ac", "-screen", "0", "1280x1024x16") - puts "Xvfb unexpectedly quit!" - end + # open Xvfb + xvfb_cmd = ["Xvfb", ":1", "-ac", "-screen", "0", "1280x1024x16"] + puts "pipeline_start for Xvfb" + pipe = IO.popen(xvfb_cmd) + @pid = pipe.pid sleep(3) # TODO: test a connection to the X server? end # disable the virtual display def disable - return @enabled = false if @existing # silent no-op if built in display + if @existing + puts "DisplayManager disable: no-op for what appears to be an existing display" + @enabled = true + return + end + return if @pid.nil? + # https://www.whatastruggle.com/timeout-a-subprocess-in-ruby begin - Process.kill 9, @pid + Timeout.timeout(30) do + Process.kill("TERM", @pid) + puts "Xvfb TERMed" + end + rescue Timeout::Error + Process.kill(9, @pid) + puts "Xvfb KILLed" ensure Process.wait @pid - @pid = nil end - puts "Xvfb killed" end # Enable a virtual display for the duration of the given block From 397e4b3c9522eab54e71a114b4bc106e5daf93ec Mon Sep 17 00:00:00 2001 From: Ian Katz Date: Thu, 11 Jan 2018 12:14:05 -0500 Subject: [PATCH 10/22] add environment to run command --- lib/arduino_ci/arduino_cmd.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/arduino_ci/arduino_cmd.rb b/lib/arduino_ci/arduino_cmd.rb index 7c8ef168..913c824f 100644 --- a/lib/arduino_ci/arduino_cmd.rb +++ b/lib/arduino_ci/arduino_cmd.rb @@ -27,7 +27,7 @@ def initialize(installation) end def run(*args) - full_args = [@installation.cmd_path] + args + full_args = [@display_mgr.environment, @installation.cmd_path] + args puts "Running $ #{full_args.join(' ')}" system(*full_args) end From 6750f6280a79125b5bb698da1afe38dfea824b7a Mon Sep 17 00:00:00 2001 From: Ian Katz Date: Thu, 11 Jan 2018 12:17:10 -0500 Subject: [PATCH 11/22] quote the string --- lib/arduino_ci/display_manager.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/arduino_ci/display_manager.rb b/lib/arduino_ci/display_manager.rb index 6eedb4ee..6ff7a2cc 100644 --- a/lib/arduino_ci/display_manager.rb +++ b/lib/arduino_ci/display_manager.rb @@ -81,7 +81,7 @@ def with_display def environment return nil unless @existing || @enabled return {} if @existing - { DISPLAY => ":1.0" } + { "DISPLAY" => ":1.0" } end # On finalize, ensure child process is ended From 1f691af171ef8a87b87491a5c29a756285e1388a Mon Sep 17 00:00:00 2001 From: Ian Katz Date: Thu, 11 Jan 2018 12:19:55 -0500 Subject: [PATCH 12/22] use builtin ruby to save development time --- .travis.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 9a3c487f..e404d04b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,12 +1,12 @@ sudo: false language: ruby -rvm: +# rvm: # - "2.0.0" # - "2.1.0" # - "2.2.0" # - rbx - - "2.5.0" +# - "2.5.0" before_install: gem install bundler -v 1.15.4 script: From 6d04a8843e6db4e031c3e1b8728d678b36a95c85 Mon Sep 17 00:00:00 2001 From: Ian Katz Date: Thu, 11 Jan 2018 12:29:02 -0500 Subject: [PATCH 13/22] better logging --- lib/arduino_ci/arduino_cmd.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/arduino_ci/arduino_cmd.rb b/lib/arduino_ci/arduino_cmd.rb index 913c824f..c0294560 100644 --- a/lib/arduino_ci/arduino_cmd.rb +++ b/lib/arduino_ci/arduino_cmd.rb @@ -27,9 +27,10 @@ def initialize(installation) end def run(*args) - full_args = [@display_mgr.environment, @installation.cmd_path] + args - puts "Running $ #{full_args.join(' ')}" - system(*full_args) + full_args = [@installation.cmd_path] + args + full_cmd = [@display_mgr.environment] + full_args + puts "Running #{@display_mgr.environment} $ #{full_args.join(' ')}" + system(*full_cmd) end def board_installed?(board) From b96fbdee60d577c9042d097e36966c50f03e7818 Mon Sep 17 00:00:00 2001 From: Ian Katz Date: Thu, 11 Jan 2018 12:29:38 -0500 Subject: [PATCH 14/22] ensure enable/disable flags are set properly --- lib/arduino_ci/display_manager.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/arduino_ci/display_manager.rb b/lib/arduino_ci/display_manager.rb index 6ff7a2cc..349033a5 100644 --- a/lib/arduino_ci/display_manager.rb +++ b/lib/arduino_ci/display_manager.rb @@ -34,24 +34,23 @@ def enable return unless @pid.nil? # TODO: disable first? - @enabled = true # open Xvfb xvfb_cmd = ["Xvfb", ":1", "-ac", "-screen", "0", "1280x1024x16"] puts "pipeline_start for Xvfb" pipe = IO.popen(xvfb_cmd) @pid = pipe.pid sleep(3) # TODO: test a connection to the X server? + @enabled = true end # disable the virtual display def disable if @existing puts "DisplayManager disable: no-op for what appears to be an existing display" - @enabled = true - return + return @enabled = false end - return if @pid.nil? + return @enabled = false if @pid.nil? # https://www.whatastruggle.com/timeout-a-subprocess-in-ruby begin @@ -64,6 +63,7 @@ def disable puts "Xvfb KILLed" ensure Process.wait @pid + @enabled = false end end From 2781f4768618314413b1b3117d60c7fbe6b687e7 Mon Sep 17 00:00:00 2001 From: Ian Katz Date: Thu, 11 Jan 2018 12:29:56 -0500 Subject: [PATCH 15/22] test output of xdpyinfo --- lib/arduino_ci/display_manager.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/arduino_ci/display_manager.rb b/lib/arduino_ci/display_manager.rb index 349033a5..299c4ae6 100644 --- a/lib/arduino_ci/display_manager.rb +++ b/lib/arduino_ci/display_manager.rb @@ -41,6 +41,8 @@ def enable @pid = pipe.pid sleep(3) # TODO: test a connection to the X server? @enabled = true + puts "\n\nxdpyinfo:\n\n" + system(environment, "xdpyinfo") end # disable the virtual display From 2a2be3c73a25a4c96f7d084c033e80fdff42164a Mon Sep 17 00:00:00 2001 From: Ian Katz Date: Thu, 11 Jan 2018 13:29:11 -0500 Subject: [PATCH 16/22] expose ability to run commands in display environment --- lib/arduino_ci/display_manager.rb | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/lib/arduino_ci/display_manager.rb b/lib/arduino_ci/display_manager.rb index 299c4ae6..c6f8bd5e 100644 --- a/lib/arduino_ci/display_manager.rb +++ b/lib/arduino_ci/display_manager.rb @@ -80,6 +80,28 @@ def with_display end end + # run a command in a display + def run(*args, **kwargs) + ret = false + # do some work to extract & merge environment variables if they exist + has_env = !args.empty? && args[0].class == Hash + with_display do |env_vars| + env_vars = {} if env_vars.nil? + env_vars.merge!(args[0]) if has_env + actual_args = has_env ? args[1..-1] : args # need to shift over if we extracted args + full_cmd = env_vars.empty? ? actual_args : [env_vars] + actual_args + + puts "Running #{env_vars} $ #{args.join(' ')}" + ret = system(*full_cmd, **kwargs) + end + ret + end + + # run a command in a display with no output + def run_silent(*args) + run(*args, out: File::NULL, err: File::NULL) + end + def environment return nil unless @existing || @enabled return {} if @existing From 342e93565ab69f692d3468d44eb28a08aed55eef Mon Sep 17 00:00:00 2001 From: Ian Katz Date: Thu, 11 Jan 2018 13:45:00 -0500 Subject: [PATCH 17/22] wrap the display manager command runner --- lib/arduino_ci/arduino_cmd.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/arduino_ci/arduino_cmd.rb b/lib/arduino_ci/arduino_cmd.rb index c0294560..e03cc97a 100644 --- a/lib/arduino_ci/arduino_cmd.rb +++ b/lib/arduino_ci/arduino_cmd.rb @@ -26,11 +26,10 @@ def initialize(installation) @installation = installation end + # run the arduino command def run(*args) full_args = [@installation.cmd_path] + args - full_cmd = [@display_mgr.environment] + full_args - puts "Running #{@display_mgr.environment} $ #{full_args.join(' ')}" - system(*full_cmd) + @display_mgr.run(*full_args) end def board_installed?(board) From b8ea020a0521ba65c13ee3064523f4402d860f26 Mon Sep 17 00:00:00 2001 From: Ian Katz Date: Thu, 11 Jan 2018 13:46:09 -0500 Subject: [PATCH 18/22] query x server to wait for launch --- lib/arduino_ci/display_manager.rb | 39 +++++++++++++++++++++++++++---- 1 file changed, 34 insertions(+), 5 deletions(-) diff --git a/lib/arduino_ci/display_manager.rb b/lib/arduino_ci/display_manager.rb index c6f8bd5e..1b398d0d 100644 --- a/lib/arduino_ci/display_manager.rb +++ b/lib/arduino_ci/display_manager.rb @@ -24,6 +24,31 @@ def existing_display? false end + # check whether a process is alive + # https://stackoverflow.com/a/32513298/2063546 + def alive?(pid) + Process.kill(0, pid) + true + rescue + false + end + + # check whether an X server is taking connections + def xserver_exist? + run_silent(nil, ["xdpyinfo"]) + end + + def xvfb_launched?(pid, timeout) + Timeout.timeout(timeout) do + loop do + return false unless alive? pid + return true if xserver_exist? + end + end + rescue Timeout::Error + false + end + # enable a virtual display def enable if @existing @@ -35,14 +60,18 @@ def enable return unless @pid.nil? # TODO: disable first? # open Xvfb - xvfb_cmd = ["Xvfb", ":1", "-ac", "-screen", "0", "1280x1024x16"] + xvfb_cmd = [ + "Xvfb", + "+extension", "RANDR", + ":1", + "-ac", + "-screen", "0", + "1280x1024x16", + ] puts "pipeline_start for Xvfb" pipe = IO.popen(xvfb_cmd) @pid = pipe.pid - sleep(3) # TODO: test a connection to the X server? - @enabled = true - puts "\n\nxdpyinfo:\n\n" - system(environment, "xdpyinfo") + @enabled = xvfb_launched?(@pid, 30) end # disable the virtual display From 46495e4a636d298e8d8ac38efcfa2a27b0f83550 Mon Sep 17 00:00:00 2001 From: Ian Katz Date: Thu, 11 Jan 2018 13:46:19 -0500 Subject: [PATCH 19/22] Add some doc comments --- lib/arduino_ci/arduino_cmd.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/arduino_ci/arduino_cmd.rb b/lib/arduino_ci/arduino_cmd.rb index e03cc97a..fdb0bdfb 100644 --- a/lib/arduino_ci/arduino_cmd.rb +++ b/lib/arduino_ci/arduino_cmd.rb @@ -9,10 +9,12 @@ class ArduinoCmd class << self protected :new + # @return [ArduinoCmd] A command object with a best guess (or nil) for the installation def autolocate new(ArduinoInstallation.autolocate) end + # @return [ArduinoCmd] A command object, installing Arduino if necessary def autolocate! new(ArduinoInstallation.autolocate!) end @@ -21,6 +23,7 @@ def autolocate! attr_accessor :installation + # @param installation [ArduinoInstallation] the location of the Arduino program installation def initialize(installation) @display_mgr = DisplayManager::instance @installation = installation From ba77f0390a0c1b962c295122e8d573f293ed01e4 Mon Sep 17 00:00:00 2001 From: Ian Katz Date: Thu, 11 Jan 2018 14:04:21 -0500 Subject: [PATCH 20/22] remove race condition in display var for environment --- lib/arduino_ci/display_manager.rb | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/lib/arduino_ci/display_manager.rb b/lib/arduino_ci/display_manager.rb index 1b398d0d..731e1540 100644 --- a/lib/arduino_ci/display_manager.rb +++ b/lib/arduino_ci/display_manager.rb @@ -2,6 +2,8 @@ require 'singleton' require 'timeout' +DESIRED_DISPLAY = ":1.0".freeze + module ArduinoCI # When arduino commands run, they need a graphical display. @@ -34,15 +36,20 @@ def alive?(pid) end # check whether an X server is taking connections - def xserver_exist? - run_silent(nil, ["xdpyinfo"]) + def xserver_exist?(display) + run_silent({ "DISPLAY" => display }, ["xdpyinfo"]) end - def xvfb_launched?(pid, timeout) + # wait for the xvfb command to launch + # @param display [String] the value of the DISPLAY env var + # @param pid [Int] the process of Xvfb + # @param timeout [Int] the timeout in seconds + # @return [Bool] whether we detected a launch + def xvfb_launched?(display, pid, timeout) Timeout.timeout(timeout) do loop do return false unless alive? pid - return true if xserver_exist? + return true if xserver_exist? display end end rescue Timeout::Error @@ -71,7 +78,7 @@ def enable puts "pipeline_start for Xvfb" pipe = IO.popen(xvfb_cmd) @pid = pipe.pid - @enabled = xvfb_launched?(@pid, 30) + @enabled = xvfb_launched?(DESIRED_DISPLAY, @pid, 30) end # disable the virtual display @@ -134,7 +141,7 @@ def run_silent(*args) def environment return nil unless @existing || @enabled return {} if @existing - { "DISPLAY" => ":1.0" } + { "DISPLAY" => DESIRED_DISPLAY } end # On finalize, ensure child process is ended From c16ebb499f23cb55246fca6a12036687e31842df Mon Sep 17 00:00:00 2001 From: Ian Katz Date: Thu, 11 Jan 2018 14:41:23 -0500 Subject: [PATCH 21/22] hail mary logging --- lib/arduino_ci/display_manager.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/arduino_ci/display_manager.rb b/lib/arduino_ci/display_manager.rb index 731e1540..0ee87c4e 100644 --- a/lib/arduino_ci/display_manager.rb +++ b/lib/arduino_ci/display_manager.rb @@ -128,6 +128,8 @@ def run(*args, **kwargs) full_cmd = env_vars.empty? ? actual_args : [env_vars] + actual_args puts "Running #{env_vars} $ #{args.join(' ')}" + puts "Full_cmd is #{full_cmd}" + puts "kwargs is #{kwargs}" ret = system(*full_cmd, **kwargs) end ret From 25e81e7ad3b83ee27490e255e600b5f96bb19c8b Mon Sep 17 00:00:00 2001 From: Ian Katz Date: Thu, 11 Jan 2018 14:44:40 -0500 Subject: [PATCH 22/22] fix xdpyinfo line --- lib/arduino_ci/display_manager.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/arduino_ci/display_manager.rb b/lib/arduino_ci/display_manager.rb index 0ee87c4e..98f2d7c8 100644 --- a/lib/arduino_ci/display_manager.rb +++ b/lib/arduino_ci/display_manager.rb @@ -37,7 +37,7 @@ def alive?(pid) # check whether an X server is taking connections def xserver_exist?(display) - run_silent({ "DISPLAY" => display }, ["xdpyinfo"]) + run_silent({ "DISPLAY" => display }, "xdpyinfo") end # wait for the xvfb command to launch