Build a shared library for use with each test (#294)

* Speed up tests by building a shared library with everything but the tests and reusing it for each test.
* Add meaningful comments and remove commented-out code.
* Set LD_LIBRARY_PATH to directory with shared library.
* Use constant variables for names and fix problem where library was not used.
* Separate rubocop from general testing in LInux.
* Rename GitHub Action workflow job.
* The path seems to be structured differently in some tests.
* Try with .dll as the library name.
* Create standalone function to build the shared library.
* rename function and give the right number of parameters
* Upshift `itoa()` result so that Windows result matches expected values.

Author: James Foster

Diff for: .github/workflows/linux.yaml

@@ -4,7 +4,7 @@ name: linux
 on: [push, pull_request]
 
 jobs:
-  "unittest_lint_sampleproject":
+  "rubocop":
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v2
@@ -17,7 +17,31 @@ jobs:
           bundle install
           bundle exec rubocop --version
           bundle exec rubocop -D .
+
+  "rspec":
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - uses: ruby/setup-ruby@v1
+        with:
+          ruby-version: 2.6
+      - name: Check style, functionality, and usage
+        run: |
+          g++ -v
+          bundle install
           bundle exec rspec --backtrace
+
+  "TestSomething":
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - uses: ruby/setup-ruby@v1
+        with:
+          ruby-version: 2.6
+      - name: TestSomething
+        run: |
+          g++ -v
+          bundle install
           cd SampleProjects/TestSomething
           bundle install
           bundle exec arduino_ci.rb

Diff for: .github/workflows/macos.yaml

@@ -4,7 +4,7 @@ name: macos
 on: [push, pull_request]
 
 jobs:
-  "unittest_lint_sampleproject":
+  "rubocop":
     runs-on: macos-latest
     steps:
       - uses: actions/checkout@v2
@@ -17,7 +17,31 @@ jobs:
           bundle install
           bundle exec rubocop --version
           bundle exec rubocop -D .
+
+  "rspec":
+    runs-on: macos-latest
+    steps:
+      - uses: actions/checkout@v2
+      - uses: ruby/setup-ruby@v1
+        with:
+          ruby-version: 2.6
+      - name: Check style, functionality, and usage
+        run: |
+          g++ -v
+          bundle install
           bundle exec rspec --backtrace
+
+  "TestSomething":
+    runs-on: macos-latest
+    steps:
+      - uses: actions/checkout@v2
+      - uses: ruby/setup-ruby@v1
+        with:
+          ruby-version: 2.6
+      - name: TestSomething
+        run: |
+          g++ -v
+          bundle install
           cd SampleProjects/TestSomething
           bundle install
           bundle exec arduino_ci.rb

Diff for: .github/workflows/windows.yaml

@@ -4,7 +4,7 @@ name: windows
 on: [push, pull_request]
 
 jobs:
-  "unittest_lint_sampleproject":
+  "rubocop_and_rspec":
     runs-on: windows-latest
     steps:
       - uses: actions/checkout@v2
@@ -17,7 +17,20 @@ jobs:
           bundle install
           bundle exec rubocop --version
           bundle exec rubocop -D .
+          echo "done with Rubocop (See https://github.com/Arduino-CI/arduino_ci/issues/315)"
           bundle exec rspec --backtrace
+
+  "TestSomething":
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - uses: ruby/setup-ruby@v1
+        with:
+          ruby-version: 2.6
+      - name: TestSomething
+        run: |
+          g++ -v
+          bundle install
           cd SampleProjects/TestSomething
           bundle install
           bundle exec arduino_ci.rb

Diff for: .gitignore

@@ -8,10 +8,14 @@ Gemfile.lock
 /spec/reports/
 vendor
 *.gem
+.arduino_ci
 
 # rspec failure tracking
 .rspec_status
 
 # C++ stuff
 *.bin
 *.bin.dSYM
+*.so
+*.so.dSYM
+.vscode

Diff for: CHANGELOG.md

@@ -14,20 +14,21 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Support for `dtostrf()`
 
 ### Changed
-- Fix copy/paste error to allow additional warnings for a platform
-- Properly report compile errors in GitHub Actions (#296)
+- We now compile a shared library to be used for each test.
 - Put build artifacts in a separate directory to reduce clutter.
 - Replace `#define yield() _NOP()` with `inline void yield() { _NOP(); }` so that other code can define a `yield()` function.
 - 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
-- Apply "rule of three" to Client copy constructor and copy assignment operator
 
 ### Deprecated
 
 ### Removed
 
 ### Fixed
+- Properly report compile errors in GitHub Actions.
+- Fix copy/paste error to allow additional warnings for a platform
+- Apply "rule of three" to Client copy constructor and copy assignment operator
 
 ### Security
 
@@ -397,7 +398,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ### Fixed
 - Malformed YAML (duplicate unittests section) now has no duplicate section
-- arduino_ci_remote.rb script now has correct arguments in build_for_test_with_configuration
+- arduino_ci_remote.rb script now has correct arguments in build_for_test
 
 
 ## [0.1.8] - 2018-04-03

Diff for: SampleProjects/TestSomething/test/stdlib.cpp

@@ -1,12 +1,13 @@
 #include <ArduinoUnitTests.h>
 #include <Arduino.h>
+#include <ctype.h>
+#include <string.h>
 
 #define ARRAY_SIZEOF(a) ( sizeof(a) / sizeof((a)[0]) )
 
 unittest(library_tests_itoa)
 {
   char buf[32];
-  const char *result;
   struct {
     int value;
     const char *expected;
@@ -26,19 +27,22 @@ unittest(library_tests_itoa)
   };
 
   for (int i = 0; i < ARRAY_SIZEOF(table); i++) {
-    result = itoa(table[i].value, buf, table[i].base);
-    assertEqual(table[i].expected, result);
+    itoa(table[i].value, buf, table[i].base);
+    for (int j = 0; j < strlen(buf); ++j) {
+      buf[j] = toupper(buf[j]);
+    }
+    assertEqual(table[i].expected, buf);
   }
 
-  // While only bases 2, 8, 10 and 16 are of real interest, lets test that all
+  // While only bases 2, 8, 10 and 16 are of real interest, let's test that all
   // bases at least produce expected output for a few test points simple to test.
   for (int base = 2; base <= 16; base++) {
-    result = itoa(0, buf, base);
-    assertEqual("0", result);
-    result = itoa(1, buf, base);
-    assertEqual("1", result);
-    result = itoa(base, buf, base);
-    assertEqual("10", result);
+    itoa(0, buf, base);
+    assertEqual("0", buf);
+    itoa(1, buf, base);
+    assertEqual("1", buf);
+    itoa(base, buf, base);
+    assertEqual("10", buf);
   }
 
 }

Diff for: exe/arduino_ci.rb

@@ -421,16 +421,16 @@ def perform_unit_tests(cpp_library, file_config)
 
   platforms.each do |p|
     puts
-    config.allowable_unittest_files(cpp_library.test_files).each do |unittest_path|
-      unittest_name = unittest_path.basename.to_s
-      compilers.each do |gcc_binary|
+    compilers.each do |gcc_binary|
+      # before compiling the tests, build a shared library of everything except the test code
+      next unless build_shared_library(gcc_binary, p, config, cpp_library)
+
+      # now build and run each test using the shared library build above
+      config.allowable_unittest_files(cpp_library.test_files).each do |unittest_path|
+        unittest_name = unittest_path.basename.to_s
+        puts "--------------------------------------------------------------------------------"
         attempt_multiline("Unit testing #{unittest_name} with #{gcc_binary} for #{p}") do
-          exe = cpp_library.build_for_test_with_configuration(
-            unittest_path,
-            config.aux_libraries_for_unittest,
-            gcc_binary,
-            config.gcc_config(p)
-          )
+          exe = cpp_library.build_for_test(unittest_path, gcc_binary)
           puts
           unless exe
             puts "Last command: #{cpp_library.last_cmd}"
@@ -445,6 +445,24 @@ def perform_unit_tests(cpp_library, file_config)
   end
 end
 
+def build_shared_library(gcc_binary, platform, config, cpp_library)
+  attempt_multiline("Build shared library with #{gcc_binary} for #{platform}") do
+    exe = cpp_library.build_shared_library(
+      config.aux_libraries_for_unittest,
+      gcc_binary,
+      config.gcc_config(platform)
+    )
+    puts
+    unless exe
+      puts "Last command: #{cpp_library.last_cmd}"
+      puts cpp_library.last_out
+      puts cpp_library.last_err
+      return false
+    end
+    return true
+  end
+end
+
 def perform_example_compilation_tests(cpp_library, config)
   phase("Compilation of example sketches")
   if @cli_options[:skip_compilation]

Diff for: lib/arduino_ci/cpp_library.rb

@@ -2,12 +2,15 @@
 require "arduino_ci/host"
 require 'pathname'
 require 'shellwords'
+require 'os'
 
 HPP_EXTENSIONS = [".hpp", ".hh", ".h", ".hxx", ".h++"].freeze
 CPP_EXTENSIONS = [".cpp", ".cc", ".c", ".cxx", ".c++"].freeze
 CI_CPP_DIR = Pathname.new(__dir__).parent.parent + "cpp"
 ARDUINO_HEADER_DIR = CI_CPP_DIR + "arduino"
 UNITTEST_HEADER_DIR = CI_CPP_DIR + "unittest"
+LIBRARY_NAME = "arduino".freeze
+BUILD_DIR = "#{Dir.pwd}/.arduino_ci".freeze    # hide build artifacts
 
 module ArduinoCI
 
@@ -464,16 +467,15 @@ def flag_args(ci_gcc_config)
       ci_gcc_config[:flags]
     end
 
-    # All GCC command line args for building any unit test
+    # All non-CPP GCC command line args for building any unit test.
+    # We leave out the CPP files so they can be included or not
+    # depending on whether we are building a shared library.
     # @param aux_libraries [Array<Pathname>] The external Arduino libraries required by this project
     # @param ci_gcc_config [Hash] The GCC config object
     # @return [Array<String>] GCC command-line flags
     def test_args(aux_libraries, ci_gcc_config)
       # TODO: something with libraries?
       ret = include_args(aux_libraries)
-      ret += cpp_files_arduino.map(&:to_s)
-      ret += cpp_files_unittest.map(&:to_s)
-      ret += cpp_files.map(&:to_s)
       unless ci_gcc_config.nil?
         cgc = ci_gcc_config
         ret = feature_args(cgc) + warning_args(cgc) + define_args(cgc) + flag_args(cgc) + ret
@@ -486,18 +488,52 @@ def test_args(aux_libraries, ci_gcc_config)
     # The dependent libraries configuration is appended with data from library.properties internal to the library under test
     #
     # @param test_file [Pathname] The path to the file containing the unit tests
+    # @param gcc_binary [String] name of a compiler
+    # @return [Pathname] path to the compiled test executable
+    def build_for_test(test_file, gcc_binary)
+      executable = Pathname.new("#{BUILD_DIR}/#{test_file.basename}.bin").expand_path
+      File.delete(executable) if File.exist?(executable)
+      arg_sets = ["-std=c++0x", "-o", executable.to_s, "-L#{BUILD_DIR}", "-DARDUINO=100"]
+      if libasan?(gcc_binary)
+        arg_sets << [ # Stuff to help with dynamic memory mishandling
+          "-g", "-O1",
+          "-fno-omit-frame-pointer",
+          "-fno-optimize-sibling-calls",
+          "-fsanitize=address"
+        ]
+      end
+      arg_sets << @test_args
+      arg_sets << [test_file.to_s, "-l#{LIBRARY_NAME}"]
+      args = arg_sets.flatten(1)
+      return nil unless run_gcc(gcc_binary, *args)
+
+      artifacts << executable
+      executable
+    end
+
+    # build a shared library to be used by each test
+    #
+    # The dependent libraries configuration is appended with data from library.properties internal to the library under test
+    #
     # @param aux_libraries [Array<Pathname>] The external Arduino libraries required by this project
+    # @param gcc_binary [String] name of a compiler
     # @param ci_gcc_config [Hash] The GCC config object
     # @return [Pathname] path to the compiled test executable
-    def build_for_test_with_configuration(test_file, aux_libraries, gcc_binary, ci_gcc_config)
-      base = test_file.basename
-      # hide build artifacts
-      build_dir = '.arduino_ci'
-      Dir.mkdir build_dir unless File.exist?(build_dir)
-      executable = Pathname.new("#{build_dir}/unittest_#{base}.bin").expand_path
+    def build_shared_library(aux_libraries, gcc_binary, ci_gcc_config)
+      Dir.mkdir BUILD_DIR unless File.exist?(BUILD_DIR)
+      if OS.windows?
+        flag = ENV["PATH"].include? ";"
+        ENV["PATH"] = BUILD_DIR + (flag ? ";" : ":") + ENV["PATH"] unless ENV["PATH"].include? BUILD_DIR
+        suffix = "dll"
+      else
+        ENV["LD_LIBRARY_PATH"] = BUILD_DIR
+        suffix = "so"
+      end
+      full_lib_name = "#{BUILD_DIR}/lib#{LIBRARY_NAME}.#{suffix}"
+      executable = Pathname.new(full_lib_name).expand_path
       File.delete(executable) if File.exist?(executable)
-      arg_sets = []
-      arg_sets << ["-std=c++0x", "-o", executable.to_s, "-DARDUINO=100"]
+      arg_sets = ["-std=c++0x", "-shared", "-fPIC", "-Wl,-undefined,dynamic_lookup",
+                  "-o", executable.to_s, "-L#{BUILD_DIR}", "-DARDUINO=100"]
       if libasan?(gcc_binary)
         arg_sets << [ # Stuff to help with dynamic memory mishandling
           "-g", "-O1",
@@ -509,10 +545,15 @@ def build_for_test_with_configuration(test_file, aux_libraries, gcc_binary, ci_g
 
       # combine library.properties defs (if existing) with config file.
       # TODO: as much as I'd like to rely only on the properties file(s), I think that would prevent testing 1.0-spec libs
-      full_dependencies = all_arduino_library_dependencies!(aux_libraries)
-      arg_sets << test_args(full_dependencies, ci_gcc_config)
-      arg_sets << cpp_files_libraries(full_dependencies).map(&:to_s)
-      arg_sets << [test_file.to_s]
+      # the following two take some time, so are cached when we build the shared library
+      @full_dependencies = all_arduino_library_dependencies!(aux_libraries)
+      @test_args = test_args(@full_dependencies, ci_gcc_config) # build full set of include directories to be cached for later
+
+      arg_sets << @test_args
+      arg_sets << cpp_files_arduino.map(&:to_s)  # Arduino.cpp, Godmode.cpp, and stdlib.cpp
+      arg_sets << cpp_files_unittest.map(&:to_s) # ArduinoUnitTests.cpp
+      arg_sets << cpp_files.map(&:to_s) # CPP files for the primary application library under test
+      arg_sets << cpp_files_libraries(@full_dependencies).map(&:to_s) # CPP files for all the libraries we depend on
       args = arg_sets.flatten(1)
       return nil unless run_gcc(gcc_binary, *args)