Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Format forcing does not bypass valid file check #125

Merged
merged 10 commits into from
May 20, 2024
66 changes: 60 additions & 6 deletions spec/coverage_reporter/parser_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")

Expand All @@ -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
Expand Down
33 changes: 24 additions & 9 deletions spec/coverage_reporter/parsers/coveragepy_parser_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion spec/coverage_reporter/parsers/lcov_parser_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
28 changes: 11 additions & 17 deletions src/coverage_reporter/parser.cr
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion src/coverage_reporter/parsers/base_parser.cr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 0 additions & 5 deletions src/coverage_reporter/parsers/cobertura_parser.cr
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
33 changes: 21 additions & 12 deletions src/coverage_reporter/parsers/coveragepy_parser.cr
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ module CoverageReporter
end

valid_file_exists && check_for_coverage_executable
rescue error : ParserError
raise error
Comment on lines +28 to +29
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

rescue Exception
false
end
Expand All @@ -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
Expand All @@ -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
3 changes: 0 additions & 3 deletions src/coverage_reporter/parsers/coveralls_parser.cr
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,6 @@ module CoverageReporter
property source_files : Array(SourceFiles)
end

def initialize(@base_path : String?)
end

def globs : Array(String)
[
"coveralls.json",
Expand Down
4 changes: 0 additions & 4 deletions src/coverage_reporter/parsers/lcov_parser.cr
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Loading