Skip to content

Conversation

@Ryuta1005
Copy link
Contributor

Summary

This PR adds two new columns — Statement Coverage and Branch Coverage — to the HTML coverage report layout.
These columns appear only when branch coverage is enabled (using the --branch option).
The goal is to provide a concrete basis for discussion in Issue #2081.
If these two columns are considered useful, this change could be merged as-is.

Changes

Preview

Before

image

After

image

Related Issue

Discussion base: #2081

@Ryuta1005
Copy link
Contributor Author

Note about the py314t / py315t CI jobs:

On macOS CI, both py314t and py315t are currently red due to failures in:

  • tests/test_concurrency.py::test_coverage_stop_in_threads

For example:

assert has_started_coverage == [t.ident]
E       assert [] == [6126907392]

and:

assert has_stopped_coverage == [t.ident]
E       assert [] == [6192033792]

This looks like a timing / threading issue specific to the free-threaded (nogil) builds and the tracer, rather than something caused by the HTML report changes in this PR.

Locally, I ran:

make test

with py313t, py314t, and py315t available via pyenv, and all test environments (including the t builds) passed:

py314t: OK (...)
py315t: OK (...)
congratulations :)

If it would be helpful, I’m happy to open a separate issue to track the test_coverage_stop_in_threads behavior on free-threaded builds, but I kept this PR focused on the HTML layout only.

@nedbat
Copy link
Member

nedbat commented Nov 15, 2025

This looks like a timing / threading issue specific to the free-threaded (nogil) builds

Yes, those tests are usually ok, but occasionally fail. An improvement would be welcome if you know what to do about them.

@nedbat
Copy link
Member

nedbat commented Nov 16, 2025

Can you add some testing in test_results.py? We don't need separate tests for each property, but they should be covered by some test in there.

@nedbat
Copy link
Member

nedbat commented Nov 16, 2025

I like the way this is going. I'd like to work on the CSS styling once it lands.

@Ryuta1005
Copy link
Contributor Author

Can you add some testing in test_results.py? We don't need separate tests for each property, but they should be covered by some test in there.

Added tests in this commit:46105c6

@Ryuta1005 Ryuta1005 changed the title [Draft] Add Statement and Branch coverage columns to index.html report Add Statement and Branch coverage columns to index.html report Nov 16, 2025
@Ryuta1005
Copy link
Contributor Author

I've addressed the feedback so far (naming + tests),
and the PR is now ready for further review.

I’ve also removed the draft status.

@nedbat
Copy link
Member

nedbat commented Nov 16, 2025

Also, I wonder if we should add these new Results properties to the JSON report? I guess that could be a second PR.

@Ryuta1005
Copy link
Contributor Author

Ryuta1005 commented Nov 16, 2025

Thanks! Yes, I agree — adding these new Results properties to the JSON report would be useful.

Once this PR is merged, I can open a follow-up PR for the JSON report.

"""Return numerator/denominator for branch coverage."""
return self.n_executed_branches, self.n_branches

def _percent(self, numerator: int, denominator: int) -> float:
Copy link
Member

Choose a reason for hiding this comment

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

Great, this was what I was thinking.

numerator, denominator = self.ratio_covered
if self.n_statements == 0:
return 100.0
return self._percent(numerator, denominator)
Copy link
Member

Choose a reason for hiding this comment

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

I think we can make this simpler because if n_statements is zero, so is the denominator:
return self._percent(*self.ratio_covered). The star syntax will also simplify the other two pc_* properties.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! Addressed in this commit: 8e88313

const table_body_rows = table.querySelectorAll("tbody tr");
const no_rows = document.getElementById("no_rows");

const header_row = table.tHead ? table.tHead.rows[table.tHead.rows.length - 1] : table.rows[0];
Copy link
Member

Choose a reason for hiding this comment

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

Why are we conditionalizing on tHead and tFoot? When won't the table have a head or a foot?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I've removed the unnecessary conditionalizing as suggested.

@nedbat
Copy link
Member

nedbat commented Nov 16, 2025

Thanks for all this, I'll merge this and make some adjustments. Let me know what name I should use to credit you in the changelog and contributors files.

@nedbat nedbat merged commit 4abe253 into coveragepy:main Nov 16, 2025
44 checks passed
@nedbat
Copy link
Member

nedbat commented Nov 16, 2025

After my styling tweaks it looks like this:
image

@Ryuta1005
Copy link
Contributor Author

Thank you so much!
It’s an honor to be included.

Please credit me as: Ryuta Otsuka.

@nedbat
Copy link
Member

nedbat commented Nov 18, 2025

This is now released as part of coverage 7.12.0.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants