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.

Sign up for GitHub

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

Jump to bottom

Make the count property optional on line node for clover parser #509

Merged
merged 4 commits into from
Jul 23, 2024
Merged

Make the count property optional on line node for clover parser #509

merged 4 commits into from
Jul 23, 2024

Conversation

hjrocha
Copy link
Contributor

@hjrocha hjrocha commented Jul 18, 2024

No description provided.

@codacy-production Codacy Production
Copy link

codacy-production bot commented Jul 18, 2024 •
edited
Loading

Make the count property optional on line node for clover parser

This pull request introduces an optional count property on the line node for the CloverParser. This change allows the parser to handle Clover reports that do not include line coverage information.

  • The config.yml file in .circleci directory is updated to use the latest version of the codacy orb.
  • The CloverParser in src/main/scala/com/codacy/parsers/implementation/CloverParser.scala is updated to handle optional count property on the line node.
  • The test file test_clover_with_paths.xml in src/test/resources is updated to include a line with an empty count property.

This comment was generated by an experimental AI tool (winner of the 2024 Codacy Hackathon 'People's Choice' and 'Most Valuable' awards).

@codacy-production Codacy Production
Copy link

codacy-production bot commented Jul 18, 2024 •
edited
Loading

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
✅ +11.71% (target: -1.00%) ✅ 100.00%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (9642dc1) 939 642 68.37%
Head commit (af29ed5) 939 (+0) 752 (+110) 80.09% (+11.71%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#509) 1 1 100.00%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

🚀 Don’t miss a bit, follow what’s new on Codacy.

Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more

@hjrocha hjrocha requested a review from lolgab July 18, 2024 09:19
@hjrocha hjrocha force-pushed the fix-clover-parser branch 12 times, most recently from 804233d to f27ba39 Compare July 18, 2024 16:48
machadoit
machadoit previously approved these changes Jul 18, 2024
View reviewed changes
Copy link
Member

@machadoit machadoit left a comment

Choose a reason for hiding this comment

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

If you are confident on the change, feel free to move forward, but would check the mentioned PR and if we want to make this change.

Also, is there any ticket you can link to the PR for future reference on why we want to make the change?

.circleci/config.yml Outdated
@@ -248,13 +248,16 @@ workflows:
export PATH=$HOME/.musl/x86_64-linux-musl-native/bin:$PATH
sbt "assembly;nativeImage"
mkdir -p ~/workdir/tmp-artifacts
ls target
Copy link
Member

Choose a reason for hiding this comment

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

Are those left here on purpose?

coverage-parser/src/main/scala/com/codacy/parsers/implementation/CloverParser.scala
@@ -71,7 +71,8 @@ object CloverParser extends CoverageParser with XmlReportParser {
fileLineTags.foldLeft[Either[String, Map[Int, Int]]](Right(Map.empty[Int, Int])) {
case (left: Left[_, _], _) => left

case (Right(lines), line) if (line \@ "type") == "stmt" || (line \@ "type") == "cond" =>
case (Right(lines), line)
if ((line \@ "type") == "stmt" || (line \@ "type") == "cond") && (line \@ "count").nonEmpty =>
Copy link
Member

Choose a reason for hiding this comment

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

So you only want to consider lines that have count defined? Seems a bit weird that there is an or to check for cond, but then you added that check as well 🤔

Seems that if the intention is not to count me, that we should revert the https://github.com/codacy/codacy-coverage-reporter/pull/420/files which seems that was added to count those lines?

@hjrocha hjrocha force-pushed the fix-clover-parser branch from 794dd69 to c6c01f9 Compare July 18, 2024 17:44
@hjrocha hjrocha force-pushed the fix-clover-parser branch 10 times, most recently from bac3373 to 8c3f4fe Compare July 19, 2024 06:22
@hjrocha hjrocha force-pushed the fix-clover-parser branch from 8c3f4fe to f949f58 Compare July 19, 2024 07:30
@hjrocha hjrocha force-pushed the fix-clover-parser branch 4 times, most recently from f1dd29e to fdee928 Compare July 23, 2024 12:07
@hjrocha hjrocha force-pushed the fix-clover-parser branch from fdee928 to af29ed5 Compare July 23, 2024 12:17
@hjrocha hjrocha merged commit 48f0bec into master Jul 23, 2024
17 checks passed
@hjrocha hjrocha deleted the fix-clover-parser branch July 23, 2024 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@manufacturist manufacturist manufacturist approved these changes

@machadoit machadoit machadoit left review comments

@lolgab lolgab Awaiting requested review from lolgab

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@hjrocha @machadoit @manufacturist