feature: only files in git will be considered in the report ALA-792 #485
Conversation
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferencesYou may notice some variations in coverage metrics with the latest Coverage engine update. For more details, visit the documentation |
pedrobpereira
force-pushed
the
feature/git-only-files-report-ALA-792
branch
2 times, most recently
from
fb2db9a to
ec585a9
Compare
| treeWalk.setRecursive(true) | ||
|
|
||
| val result: Seq[String] = | ||
| if (treeWalk.next) { |
There was a problem hiding this comment.
interesting if, is it really needed? Don't the continually take care of it? Also, should this return Stream explicitly? To know that the caller needs to consume this?
There was a problem hiding this comment.
Yeah, I think it is needed, I'll test this again without, but I added it because it was failing without the first next. Since return type is typified, I don't think we have to worry about that right?, whoever is consuming will only interact with a Seq.
| private def transform( | ||
| report: CoverageReport | ||
| )(config: ReportConfig, acceptableFileNames: Seq[String]): CoverageReport = { | ||
| val transformations = Set(new PathPrefixer(config.prefix), new GitFileNameUpdaterAndFilter(acceptableFileNames)) |
There was a problem hiding this comment.
Be careful with the the Set here, probably we should change to a Seq. Since Set does not guarantee order, and we should guarantee that the GitFileNameUpdaterAndFilteris the last transformation
| files | ||
| .map { file => | ||
| logger.info(s"Parsing coverage data from: ${file.getAbsolutePath} ...") | ||
| for { | ||
| _ <- validateFileAccess(file) | ||
| files <- gitFiles |
There was a problem hiding this comment.
We can consider this for to allow for optional files maybe? So we fallback to not doing this new mechanism if anything fails
| fileReport <- report.fileReports | ||
| fileName <- FileNameMatcher | ||
| .matchAndReturnName(fileReport.filename, acceptableFileNames) | ||
| .map(Some(_)) |
There was a problem hiding this comment.
Strange code here, maybe an .orElse on the log, is that what you want?
There was a problem hiding this comment.
I think it looks bloated even with .orElse. Maybe move it outside:
[...]
fileName <- matchFileName(fileReport)
[...]
private def matchFileName(fileReport: CoverageFileReport): Option[String] = {
val maybeFileName = FileNameMatcher.matchAndReturnName(fileReport.filename, acceptableFileNames)
if (maybeFileName.isEmpty)
logger.warn(s"File: ${fileReport.filename} will be discarded and will not be considered for coverage calculation")
maybeFileName
}
pedrobpereira
force-pushed
the
feature/git-only-files-report-ALA-792
branch
7 times, most recently
from
e51096d to
3dd10db
Compare
…e will be according to git file structure ALA-792
pedrobpereira
force-pushed
the
feature/git-only-files-report-ALA-792
branch
from
3dd10db to
0794c46
Compare
|
Converted to Draft again to prevent someone merging this, we will be waiting till the 1st week of January to go ahead with this. |
file name will be according to git file structure ALA-792