Skip to content

Add handling for branch coverage information from CTest#3598

Open
josephsnyder wants to merge 1 commit intoKitware:masterfrom
josephsnyder:add_ctest_branch_coverage_handling
Open

Add handling for branch coverage information from CTest#3598
josephsnyder wants to merge 1 commit intoKitware:masterfrom
josephsnyder:add_ctest_branch_coverage_handling

Conversation

@josephsnyder
Copy link
Copy Markdown
Member

When reading the coverage log file, look for incoming "BranchesTested" and "BranchesUntested" attributes on each log line. If either is greater than 0, add a branch entry to that line.

@williamjallen williamjallen marked this pull request as draft April 8, 2026 15:33
@josephsnyder josephsnyder force-pushed the add_ctest_branch_coverage_handling branch 2 times, most recently from da1a747 to 4cf01c7 Compare April 9, 2026 15:37
@josephsnyder josephsnyder marked this pull request as ready for review April 9, 2026 15:37
@josephsnyder josephsnyder marked this pull request as draft April 9, 2026 16:29
@josephsnyder
Copy link
Copy Markdown
Member Author

Converting back to draft. The coverage display of a real submission only showed the branch information

When reading the coverage log file, look for incoming "BranchesTested"
and "BranchesUntested" attributes on each log line.  If either is greater
than 0, add a branch entry to that line.
@josephsnyder josephsnyder force-pushed the add_ctest_branch_coverage_handling branch from 4cf01c7 to c8dbdf6 Compare April 9, 2026 18:31
@josephsnyder josephsnyder marked this pull request as ready for review April 9, 2026 19:38
$this->CurrentCoverageFileLog = new CoverageFileLog();
$this->CurrentCoverageFile->FullPath = trim($attributes['FULLPATH']);
} elseif ($name === 'LINE') {
if (array_key_exists('BRANCHESTESTED', $attributes)) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Just to be safe, let's check that both BRANCHESTESTED and BRANCHESUNTESTED exist here since you use them immediately below.

Comment on lines +71 to +73
$this->CurrentCoverageFileLog->AddBranch($attributes['NUMBER'],
$attributes['BRANCHESTESTED'],
$attributes['BRANCHESTESTED'] + $attributes['BRANCHESUNTESTED']);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
$this->CurrentCoverageFileLog->AddBranch($attributes['NUMBER'],
$attributes['BRANCHESTESTED'],
$attributes['BRANCHESTESTED'] + $attributes['BRANCHESUNTESTED']);
$this->CurrentCoverageFileLog->AddBranch($attributes['NUMBER'],
$attributes['BRANCHESTESTED'],
$attributes['BRANCHESTESTED'] + $attributes['BRANCHESUNTESTED'],
);

Comment on lines +71 to +73
$this->CurrentCoverageFileLog->AddBranch($attributes['NUMBER'],
$attributes['BRANCHESTESTED'],
$attributes['BRANCHESTESTED'] + $attributes['BRANCHESUNTESTED']);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

You could also cast these to ints to be more clear.

<File Name="hello.cxx" FullPath="./hello.cxx">
<Report>

<Line Number="3" Count="66" BranchesTested="2" BranchesUntested="0"> for (int i =0; i &lt; times; i++) {</Line>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Question: Does the whitespace after <Report> exist in the CMake-generated files? Ditto for the whitespace before for ....

branchesUntested
coveredLines {
branchesHit
totalBranches
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can you also select the line number here so the test also verifies that the right line number is associated with the branch coverage?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants