Obey 1.0 / 1.5 library specification when finding C++ library source

Author: ianfixes

Diff for: CHANGELOG.md

@@ -8,9 +8,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 ## [Unreleased]
 ### Added
 - `arduino_ci_remote.rb` CLI switch `--skip-examples-compilation`
+- `CppLibrary.header_files` to find header files
 
 ### Changed
 - Move repository from https://github.com/ianfixes/arduino_ci to https://github.com/Arduino-CI/arduino_ci
+- `CppLibrary` functions returning C++ header or code files now respect the 1.0/1.5 library specification
 
 ### Deprecated
 - `arduino_ci_remote.rb` CLI switch `--skip-compilation`

Diff for: SampleProjects/OnePointFiveDummy/NoBase.cpp

@@ -0,0 +1 @@
+This project resembles a "1.5 spec" library: it has `library.properties` and a `src/` directory that will be scanned recursively.  `utility/`, if present, will be ignored.

Diff for: SampleProjects/OnePointFiveDummy/NoBase.h

@@ -0,0 +1 @@
+This project lacks a `library.properties` and so should be treated as a "1.0 spec" library -- the base and `utility` directories will be scanned for code, non-recursively.  `src/`, if present, will be ignored.

Diff for: SampleProjects/OnePointFiveDummy/README.md

@@ -0,0 +1 @@
+This project should resemble "1.0 spec" library -- the base and `utility` directories will be scanned for code, non-recursively.  `src/`, if present, will be ignored.

Diff for: SampleProjects/OnePointFiveDummy/library.properties

@@ -1,7 +1,13 @@
 Arduino Sample Projects
 =======================
 
-This directory contains projects that are meant to be built with and tested by this gem. Although this directory is named `SampleProjects`, it is by no means optional. These project test the testing framework itself, but also provide examples of how you might write your own tests (which should be placed in your system's Arduino `libraries` directory). 
+This directory contains projects that are intended solely for testing the various features of this gem -- to test the testing framework itself.  The RSpec tests refer specifically to these projects.
 
-* "DoSomething" is a simple test of the testing framework (arduino_ci) itself to verfy that passes and failures are properly identified and reported.
-* "TestSomething" contains tests for all the mock features of arduino_ci.
+Because of this, these projects include some intentional quirks that differ from what a well-formed an Arduino project for testing with `arduino_ci` might contain.  See other projects in the "Arduino-CI" GitHub organization for practical examples.
+
+
+* "TestSomething" contains a minimial library, but tests for all the C++ compilation feature-mocks of arduino_ci.
+* "DoSomething" is a simple test of the testing framework (arduino_ci) itself to verfy that passes and failures are properly identified and reported.  Because of this, it includes test files that are expected to fail -- they are prefixed with "bad-".
+* "OnePointOhDummy" is a non-functional library meant to test file inclusion logic on libraries conforming to the "1.0" specification
+* "OnePointFiveMalformed" is a non-functional library meant to test file inclusion logic on libraries that attempt to conform to the ["1.5" specfication](https://arduino.github.io/arduino-cli/latest/library-specification/) but fail to include a `src` directory
+* "OnePointFiveDummy" is a non-functional library meant to test file inclusion logic on libraries conforming to the ["1.5" specfication](https://arduino.github.io/arduino-cli/latest/library-specification/)

Diff for: SampleProjects/OnePointFiveDummy/src/YesSrc.cpp

@@ -55,6 +55,18 @@ def initialize(base_dir, arduino_lib_dir, exclude_dirs)
       @vendor_bundle_cache = nil
     end
 
+    # Decide whether this is a 1.5-compatible library
+    #
+    # according to https://arduino.github.io/arduino-cli/latest/library-specification
+    #
+    # Should match logic from https://github.com/arduino/arduino-cli/blob/master/arduino/libraries/loader.go
+    # @return [bool]
+    def one_point_five?
+      lib_props = (@base_dir + "library.properties")
+      src_dir = (@base_dir + "src")
+      [lib_props, src_dir].all?(&:exist?) && lib_props.file? && src_dir.directory?
+    end
+
     # Guess whether a file is part of the vendor bundle (indicating we should ignore it).
     #
     # A safe way to do this seems to be to check whether any of the installed gems
@@ -152,41 +164,76 @@ def libasan?(gcc_binary)
 
     # Get a list of all CPP source files in a directory and its subdirectories
     # @param some_dir [Pathname] The directory in which to begin the search
+    # @param extensions [Array<Sring>] The set of allowable file extensions
     # @return [Array<Pathname>] The paths of the found files
-    def cpp_files_in(some_dir)
+    def code_files_in(some_dir, extensions)
       raise ArgumentError, 'some_dir is not a Pathname' unless some_dir.is_a? Pathname
       return [] unless some_dir.exist? && some_dir.directory?
 
       real = some_dir.realpath
-      files = Find.find(real).map { |p| Pathname.new(p) }.reject(&:directory?)
-      cpp = files.select { |path| CPP_EXTENSIONS.include?(path.extname.downcase) }
+      files = some_dir.realpath.children.reject(&:directory?)
+      cpp = files.select { |path| extensions.include?(path.extname.downcase) }
       not_hidden = cpp.reject { |path| path.basename.to_s.start_with?(".") }
       not_hidden.sort_by(&:to_s)
     end
 
+    # Get a list of all CPP source files in a directory and its subdirectories
+    # @param some_dir [Pathname] The directory in which to begin the search
+    # @param extensions [Array<Sring>] The set of allowable file extensions
+    # @return [Array<Pathname>] The paths of the found files
+    def code_files_in_recursive(some_dir, extensions)
+      raise ArgumentError, 'some_dir is not a Pathname' unless some_dir.is_a? Pathname
+      return [] unless some_dir.exist? && some_dir.directory?
+
+      real = some_dir.realpath
+      Find.find(real).map { |p| Pathname.new(p) }.select(&:directory?).map { |d| code_files_in(d, extensions) }.flatten
+    end
+
+    # Header files that are part of the project library under test
+    # @return [Array<Pathname>]
+    def header_files
+      ret = if one_point_five?
+        code_files_in_recursive(@base_dir + "src", HPP_EXTENSIONS)
+      else
+        [@base_dir, @base_dir + "utility"].map { |d| code_files_in(d, HPP_EXTENSIONS) }.flatten
+      end
+
+      # note to future troubleshooter: some of these tests may not be relevant, but at the moment at
+      # least some of them are tied to existing features
+      ret.reject { |p| vendor_bundle?(p) || in_tests_dir?(p) || in_exclude_dir?(p) }
+    end
+
     # CPP files that are part of the project library under test
     # @return [Array<Pathname>]
     def cpp_files
-      cpp_files_in(@base_dir).reject { |p| vendor_bundle?(p) || in_tests_dir?(p) || in_exclude_dir?(p) }
+      ret = if one_point_five?
+        code_files_in_recursive(@base_dir + "src", CPP_EXTENSIONS)
+      else
+        [@base_dir, @base_dir + "utility"].map { |d| code_files_in(d, CPP_EXTENSIONS) }.flatten
+      end
+
+      # note to future troubleshooter: some of these tests may not be relevant, but at the moment at
+      # least some of them are tied to existing features
+      ret.reject { |p| vendor_bundle?(p) || in_tests_dir?(p) || in_exclude_dir?(p) }
     end
 
     # CPP files that are part of the arduino mock library we're providing
     # @return [Array<Pathname>]
     def cpp_files_arduino
-      cpp_files_in(ARDUINO_HEADER_DIR)
+      code_files_in(ARDUINO_HEADER_DIR, CPP_EXTENSIONS)
     end
 
     # CPP files that are part of the unit test library we're providing
     # @return [Array<Pathname>]
     def cpp_files_unittest
-      cpp_files_in(UNITTEST_HEADER_DIR)
+      code_files_in(UNITTEST_HEADER_DIR, CPP_EXTENSIONS)
     end
 
     # CPP files that are part of the 3rd-party libraries we're including
     # @param [Array<String>] aux_libraries
     # @return [Array<Pathname>]
     def cpp_files_libraries(aux_libraries)
-      arduino_library_src_dirs(aux_libraries).map { |d| cpp_files_in(d) }.flatten.uniq
+      arduino_library_src_dirs(aux_libraries).map { |d| code_files_in(d, CPP_EXTENSIONS) }.flatten.uniq
     end
 
     # Returns the Pathnames for all paths to exclude from testing and compilation
@@ -204,15 +251,13 @@ def tests_dir
     # The files provided by the user that contain unit tests
     # @return [Array<Pathname>]
     def test_files
-      cpp_files_in(tests_dir)
+      code_files_in(tests_dir, CPP_EXTENSIONS)
     end
 
     # Find all directories in the project library that include C++ header files
     # @return [Array<Pathname>]
     def header_dirs
-      real = @base_dir.realpath
-      all_files = Find.find(real).map { |f| Pathname.new(f) }.reject(&:directory?)
-      unbundled = all_files.reject { |path| vendor_bundle?(path) }
+      unbundled = header_files.reject { |path| vendor_bundle?(path) }
       unexcluded = unbundled.reject { |path| in_exclude_dir?(path) }
       files = unexcluded.select { |path| HPP_EXTENSIONS.include?(path.extname.downcase) }
       files.map(&:dirname).uniq
@@ -241,15 +286,8 @@ def gcc_version(gcc_binary)
     def arduino_library_src_dirs(aux_libraries)
       # Pull in all possible places that headers could live, according to the spec:
       # https://github.com/arduino/Arduino/wiki/Arduino-IDE-1.5:-Library-specification
-      # TODO: be smart and implement library spec (library.properties, etc)?
-      subdirs = ["", "src", "utility"]
-      all_aux_include_dirs_nested = aux_libraries.map do |libdir|
-        # library manager coerces spaces in package names to underscores
-        # see https://github.com/Arduino-CI/arduino_ci/issues/132#issuecomment-518857059
-        legal_libdir = libdir.tr(" ", "_")
-        subdirs.map { |subdir| Pathname.new(@arduino_lib_dir) + legal_libdir + subdir }
-      end
-      all_aux_include_dirs_nested.flatten.select(&:exist?).select(&:directory?)
+
+      aux_libraries.map { |d| self.new(d, @arduino_lib_dir, @exclude_dirs).header_dirs }.flatten
     end
 
     # GCC command line arguments for including aux libraries

Diff for: SampleProjects/OnePointFiveDummy/src/YesSrc.h

@@ -12,47 +12,101 @@ def get_relative_dir(sampleprojects_tests_dir)
 
 RSpec.describe ArduinoCI::CppLibrary do
   next if skip_ruby_tests
-  cpp_lib_path = sampleproj_path + "DoSomething"
-  cpp_library = ArduinoCI::CppLibrary.new(cpp_lib_path, Pathname.new("my_fake_arduino_lib_dir"), [])
-  context "cpp_files" do
-    it "finds cpp files in directory" do
-      dosomething_cpp_files = [Pathname.new("DoSomething") + "do-something.cpp"]
-      relative_paths = cpp_library.cpp_files.map { |f| get_relative_dir(f) }
-      expect(relative_paths).to match_array(dosomething_cpp_files)
-    end
-  end
-
-  context "header_dirs" do
-    it "finds directories containing h files" do
-      dosomething_header_dirs = [Pathname.new("DoSomething")]
-      relative_paths = cpp_library.header_dirs.map { |f| get_relative_dir(f) }
-      expect(relative_paths).to match_array(dosomething_header_dirs)
-    end
-  end
-
-  context "tests_dir" do
-    it "locates the tests directory" do
-      # since we don't know where the CI system will install this stuff,
-      # we need to go looking for a relative path to the SampleProjects directory
-      # just to get our "expected" value
-      relative_path = get_relative_dir(cpp_library.tests_dir)
-      expect(relative_path.to_s).to eq("DoSomething/test")
-    end
-  end
 
-  context "test_files" do
-    it "finds cpp files in directory" do
-      dosomething_test_files = [
+  answers = {
+    "DoSomething": {
+      one_five: false,
+      cpp_files: [Pathname.new("DoSomething") + "do-something.cpp"],
+      header_dirs: [Pathname.new("DoSomething")],
+      test_files: [
         "DoSomething/test/good-null.cpp",
         "DoSomething/test/good-library.cpp",
         "DoSomething/test/bad-null.cpp",
       ].map { |f| Pathname.new(f) }
-      relative_paths = cpp_library.test_files.map { |f| get_relative_dir(f) }
-      expect(relative_paths).to match_array(dosomething_test_files)
+    },
+    "OnePointOhDummy": {
+      one_five: false,
+      cpp_files: [
+        "OnePointOhDummy/YesBase.cpp",
+        "OnePointOhDummy/utility/YesUtil.cpp",
+      ].map { |f| Pathname.new(f) },
+      header_dirs: [
+        "OnePointOhDummy",
+        "OnePointOhDummy/utility"
+      ].map { |f| Pathname.new(f) },
+      test_files: []
+    },
+    "OnePointFiveMalformed": {
+      one_five: false,
+      cpp_files: [
+        "OnePointFiveMalformed/YesBase.cpp",
+        "OnePointFiveMalformed/utility/YesUtil.cpp",
+      ].map { |f| Pathname.new(f) },
+      header_dirs: [
+        "OnePointFiveMalformed",
+        "OnePointFiveMalformed/utility"
+      ].map { |f| Pathname.new(f) },
+      test_files: []
+    },
+    "OnePointFiveDummy": {
+      one_five: true,
+      cpp_files: [
+        "OnePointFiveDummy/src/YesSrc.cpp",
+        "OnePointFiveDummy/src/subdir/YesSubdir.cpp",
+      ].map { |f| Pathname.new(f) },
+      header_dirs: [
+        "OnePointFiveDummy/src",
+        "OnePointFiveDummy/src/subdir",
+      ].map { |f| Pathname.new(f) },
+      test_files: []
+    }
+  }.freeze
+
+  answers.each do |sampleproject, expected|
+    context "#{sampleproject}" do
+      cpp_lib_path = sampleproj_path + sampleproject.to_s
+      cpp_library = ArduinoCI::CppLibrary.new(cpp_lib_path, Pathname.new("my_fake_arduino_lib_dir"), [])
+
+      it "detects 1.5 format" do
+        expect(cpp_library.one_point_five?).to eq(expected[:one_five])
+      end
+
+      context "cpp_files" do
+        it "finds cpp files in directory" do
+          relative_paths = cpp_library.cpp_files.map { |f| get_relative_dir(f) }
+          expect(relative_paths.map(&:to_s)).to match_array(expected[:cpp_files].map(&:to_s))
+        end
+      end
+
+      context "header_dirs" do
+        it "finds directories containing h files" do
+          relative_paths = cpp_library.header_dirs.map { |f| get_relative_dir(f) }
+          expect(relative_paths.map(&:to_s)).to match_array(expected[:header_dirs].map(&:to_s))
+        end
+      end
+
+      context "tests_dir" do
+        it "locates the tests directory" do
+          # since we don't know where the CI system will install this stuff,
+          # we need to go looking for a relative path to the SampleProjects directory
+          # just to get our "expected" value
+          relative_path = get_relative_dir(cpp_library.tests_dir)
+          expect(relative_path.to_s).to eq("#{sampleproject}/test")
+        end
+      end
+
+      context "test_files" do
+        it "finds cpp files in directory" do
+          relative_paths = cpp_library.test_files.map { |f| get_relative_dir(f) }
+          expect(relative_paths.map(&:to_s)).to match_array(expected[:test_files].map(&:to_s))
+        end
+      end
     end
   end
 
   context "test" do
+    cpp_lib_path = sampleproj_path + "DoSomething"
+    cpp_library = ArduinoCI::CppLibrary.new(cpp_lib_path, Pathname.new("my_fake_arduino_lib_dir"), [])
     config = ArduinoCI::CIConfig.default
 
     after(:each) do |example|