Format forcing does not bypass valid file check (#125)

When forcing a format, the system still collects all possible
coverage files across all file types, and passes them directly
to the specified parser, bypassing the parser's matches? method
that checks to make sure that the file is of a valid format. This
is causing issues when a user is forcing the cobertura format, 
but has a .coverage file without the coverage executable installed.

These changes ensure that if a user specifies a format, then
only the files that match the glob-pattern for the specified
parser will be passed to the parser. In addition, the matches?
method is no longer bypassed, which ensures that even if a
file matches the correct naming pattern, there will be no attempt
to parse it if the actual file format is incorrect.

If a user specifies the python format for a .coverage file, but does
not have the coverage executable installed, then an error will be
raised. If no format is specified, and the user has a .coverage file
but does not have the coverage executable installed, then a debug
warning message only will be displayed.

spec/coverage_reporter/parser_spec.cr

@@ -46,7 +46,7 @@ Spectator.describe CoverageReporter::Parser do
         end
 
         context "when a file is specified" do
-          let(coverage_files) { ["spec/fixtures/lcov/for-base-path-lcov"] }
+          let(coverage_files) { ["spec/fixtures/lcov/for-base-path.lcov"] }
           let(base_path) { "spec/fixtures/lcov" }
 
           it "returns report only for specified file" do
@@ -83,9 +83,9 @@ Spectator.describe CoverageReporter::Parser do
 
         context "when a file is specified and coverage is not installed" do
           let(coverage_files) { ["spec/fixtures/python/.coverage"] }
-          let(base_path) { "spec/fixtures/lcov" }
+          let(base_path) { "spec/fixtures/python" }
 
-          it "returns report only for specified file" do
+          it "raises error" do
             path = ENV["PATH"]
             ENV.delete("PATH")
 
@@ -107,10 +107,64 @@ Spectator.describe CoverageReporter::Parser do
     end
 
     context "for all files" do
-      it "returns reports for all files" do
-        reports = subject.parse
+      context "when coveragepy is installed" do
+        it "returns reports for all files" do
+          reports = subject.parse
+
+          expect(reports.size).to be > 2
+        end
+      end
+
+      context "when coveragepy is not installed" do
+        it "returns reports for all files (no error is raised)" do
+          path = ENV["PATH"]
+          ENV.delete("PATH")
+
+          reports = subject.parse
+
+          expect(reports.size).to be > 2
+
+          ENV["PATH"] = path
+        end
+      end
+    end
+  end
+
+  describe "#files" do
+    context "when no coverage_format specified" do
+      it "returns all possible files across all formats" do
+        files = subject.files
+
+        expect(files).to match_array [
+          "spec/fixtures/lcov/coverage/test.lcov",
+          "spec/fixtures/lcov/test.lcov",
+          "spec/fixtures/lcov/test-current-folder.lcov",
+          "spec/fixtures/lcov/empty.lcov",
+          "spec/fixtures/lcov/for-base-path.lcov",
+          "spec/fixtures/simplecov/.resultset.json",
+          "spec/fixtures/clover/clover.xml",
+          "spec/fixtures/cobertura/cobertura.xml",
+          "spec/fixtures/cobertura/cobertura-coverage.xml",
+          "spec/fixtures/jacoco/jacoco-oneline-report.xml",
+          "spec/fixtures/jacoco/jacoco-report-multiple-packages.xml",
+          "spec/fixtures/jacoco/jacoco-report.xml",
+          "spec/fixtures/gcov/main.c.gcov",
+          "spec/fixtures/python/.coverage",
+          "spec/fixtures/coveralls/coveralls.json",
+        ]
+      end
+    end
+
+    context "when coverage_format specified" do
+      let(coverage_format) { "cobertura" }
+
+      it "only returns possible files for the specified format" do
+        files = subject.files
 
-        expect(reports.size).to be > 2
+        expect(files).to match_array [
+          "spec/fixtures/cobertura/cobertura.xml",
+          "spec/fixtures/cobertura/cobertura-coverage.xml",
+        ]
       end
     end
   end

spec/coverage_reporter/parsers/coveragepy_parser_spec.cr

@@ -4,19 +4,34 @@ Spectator.describe CoverageReporter::CoveragepyParser do
   subject { described_class.new(nil) }
 
   describe "#matches?" do
-    it "matches only SQLite3 db file" do
-      expect(subject.matches?("spec/fixtures/python/.coverage")).to eq true
-      expect(subject.matches?("spec/fixtures/python/.coverage-non-existing")).to eq false
-      expect(subject.matches?("spec/fixtures/golang/coverage.out")).to eq false
+    context "when format is not forced" do
+      it "matches only SQLite3 db file" do
+        expect(subject.matches?("spec/fixtures/python/.coverage")).to eq true
+        expect(subject.matches?("spec/fixtures/python/.coverage-non-existing")).to eq false
+        expect(subject.matches?("spec/fixtures/golang/coverage.out")).to eq false
+      end
+
+      it "does not match if coverage program is not installed" do
+        path = ENV["PATH"]
+        ENV.delete("PATH")
+
+        expect(subject.matches?("spec/fixtures/python/.coverage")).to be_falsey
+
+        ENV["PATH"] = path
+      end
     end
 
-    it "does not match if coverage program is not installed" do
-      path = ENV["PATH"]
-      ENV.delete("PATH")
+    context "when format is forced" do
+      subject { described_class.new(nil, true) }
+
+      it "raises error if coverage program is not installed" do
+        path = ENV["PATH"]
+        ENV.delete("PATH")
 
-      expect(subject.matches?("spec/fixtures/python/.coverage")).to be_falsey
+        expect { subject.matches?("spec/fixtures/python/.coverage") }.to raise_error(CoverageReporter::CoveragepyParser::ParserError)
 
-      ENV["PATH"] = path
+        ENV["PATH"] = path
+      end
     end
   end
 

spec/coverage_reporter/parsers/lcov_parser_spec.cr

@@ -61,7 +61,7 @@ Spectator.describe CoverageReporter::LcovParser do
     end
 
     context "with base path" do
-      let(filename) { "spec/fixtures/lcov/for-base-path-lcov" }
+      let(filename) { "spec/fixtures/lcov/for-base-path.lcov" }
       let(base_path) { "spec/fixtures/lcov" }
 
       it "parses correctly" do

src/coverage_reporter/parser.cr

@@ -49,7 +49,17 @@ module CoverageReporter
     end
 
     def initialize(@coverage_files : Array(String) | Nil, @coverage_format : String?, @base_path : String?)
-      @parsers = PARSERS.map(&.new(@base_path)).to_a
+      @parsers = if coverage_format
+                   Log.info("✏️ Forced coverage format: #{coverage_format}")
+                   parser_class = PARSERS.find { |klass| klass.name == coverage_format }
+                   if parser_class
+                     [parser_class.new(base_path, true)]
+                   else
+                     raise InvalidCoverageFormat.new(coverage_format)
+                   end
+                 else
+                   PARSERS.map(&.new(base_path)).to_a
+                 end
     end
 
     # Returns coverage report files that can be parsed by utility.
@@ -79,22 +89,6 @@ module CoverageReporter
     end
 
     def parse : SourceFiles
-      if coverage_format
-        Log.info("✏️ Forced coverage format: #{coverage_format}")
-        parser_class = PARSERS.find { |klass| klass.name == coverage_format }
-        if parser_class
-          parser = parser_class.new(base_path)
-          source_files = SourceFiles.new
-          files.each do |filename|
-            source_files.add(parser.parse(filename), filename)
-          end
-
-          return source_files
-        else
-          raise InvalidCoverageFormat.new(coverage_format)
-        end
-      end
-
       source_files = SourceFiles.new
       files.each do |filename|
         source_files.add(parse_file(filename), filename)

src/coverage_reporter/parsers/base_parser.cr

@@ -45,7 +45,7 @@ module CoverageReporter
     #
     # *base_path* can be used to join with all paths in coverage report in order
     # to properly reference a file.
-    def initialize(@base_path : String? = nil)
+    def initialize(@base_path : String? = nil, @forced : Bool = false)
     end
 
     # Creates a `FileReport` instance. Use this method instead of explicit

src/coverage_reporter/parsers/cobertura_parser.cr

@@ -7,11 +7,6 @@ module CoverageReporter
       coverage : Hash(Line, Hits?),
       branches : Hash(Line, Array(Hits))
 
-    # NOTE: Provide the base path for the sources. You can check "filename" in
-    #       coverage report and see what part is missing to get a valid source path.
-    def initialize(@base_path : String?)
-    end
-
     def globs : Array(String)
       [
         "**/*/cobertura.xml",

src/coverage_reporter/parsers/coveragepy_parser.cr

@@ -25,6 +25,8 @@ module CoverageReporter
       end
 
       valid_file_exists && check_for_coverage_executable
+    rescue error : ParserError
+      raise error
     rescue Exception
       false
     end
@@ -41,17 +43,8 @@ module CoverageReporter
         parser = CoberturaParser.new(@base_path)
         parser.parse(tmpfile.path)
       else
-        error_message =
-          %Q|There was an error processing #{filename}: #{error}
+        error_message = "There was an error processing #{filename}: #{error}\n\n#{missing_coverage_executable_message}"
 
-To use the #{self.class.name} format, do one of the following:
-1. Make sure that the coverage executable is available in the
-   runner environment, or
-2. Convert the .coverage file to a coverage.xml file by running
-   `coverage xml`. Then pass the input option `format: cobertura`
-   (for Coveralls GitHub Action or orb), or pass `--format=cobertura`
-   if using the coverage reporter alone.
-|
         raise ParserError.new(error_message)
       end
     ensure
@@ -72,9 +65,25 @@ To use the #{self.class.name} format, do one of the following:
       if process_status.success?
         true
       else
-        Log.debug("⚠️  Detected coverage format: #{self.class.name}, but error with coverage executable: #{error}")
-        false
+        if @forced
+          raise ParserError.new(missing_coverage_executable_message)
+        else
+          Log.debug("⚠️  Detected coverage format: #{self.class.name}, but error with coverage executable: #{error}")
+          Log.debug(missing_coverage_executable_message)
+          false
+        end
       end
     end
+
+    private def missing_coverage_executable_message
+      %Q|To use the #{self.class.name} format, do one of the following:
+1. Make sure that the coverage executable is available in the
+   runner environment, or
+2. Convert the .coverage file to a coverage.xml file by running
+   `coverage xml`. Then pass the input option `format: cobertura`
+   (for Coveralls GitHub Action or orb), or pass `--format=cobertura`
+   if using the coverage reporter alone.
+|
+    end
   end
 end

src/coverage_reporter/parsers/coveralls_parser.cr

@@ -17,9 +17,6 @@ module CoverageReporter
       property source_files : Array(SourceFiles)
     end
 
-    def initialize(@base_path : String?)
-    end
-
     def globs : Array(String)
       [
         "coveralls.json",

src/coverage_reporter/parsers/lcov_parser.cr

@@ -13,10 +13,6 @@ module CoverageReporter
     BRANCH_COVERAGE_RE = Regex.new("\\ABRDA:(\\d+),(\\d+),(\\d+),(\\d+|-)", Regex::CompileOptions::MATCH_INVALID_UTF)
     END_RE             = Regex.new("\\Aend_of_record", Regex::CompileOptions::MATCH_INVALID_UTF)
 
-    # Use *base_path* to join with paths found in reports.
-    def initialize(@base_path : String?)
-    end
-
     def globs : Array(String)
       [
         "*.lcov",