Skip to content

Fix: correct logical condition for covered branches #2089

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

5angjun
Copy link

@5angjun 5angjun commented Jul 4, 2025

Summary

This PR fixes the logic in the extract_covered_branches_from_summary_json function in coverage_utils.py.

The goal is to ensure that only branches which are:

  • of type 4 (branch region), and
  • actually covered (hit_true > 0 or hit_false > 0)
    are included in the result.

Notes

  • Fixes a logical error caused by incorrect operator precedence.
  • Previously, branches with type != 4 could be incorrectly included if hit_true > 0.

Test Cases

import pytest
from unittest.mock import patch
from experiment.measurer.coverage_utils import extract_covered_branches_from_summary_json

@pytest.fixture
def fake_coverage_data():
    return {
        "data": [
            {
                "functions": [
                    {
                        "branches": [
                            # type == 4, hit_true > 0 → should be included
                            [10, 5, 10, 15, 1, 0, 0, 0, 4],
                            # type == 4, hit_false > 0 → should be included
                            [11, 5, 11, 15, 0, 2, 0, 0, 4],
                            # type == 4, not covered → should be excluded
                            [12, 5, 12, 15, 0, 0, 0, 0, 4],
                            # type != 4 → should be excluded
                            [13, 5, 13, 15, 1, 0, 0, 0, 2],
                            # type == 4, both hit_true and hit_false > 0 → should be included
                            [14, 5, 14, 15, 3, 3, 0, 0, 4]
                        ]
                    }
                ]
            }
        ]
    }

@patch('experiment.measurer.coverage_utils.get_coverage_infomation')
def test_extract_covered_branches_from_summary_json(mock_get_info, fake_coverage_data):
    mock_get_info.return_value = fake_coverage_data

    result = extract_covered_branches_from_summary_json("dummy.json")

    # Expecting 3 valid covered branches (type == 4 and covered)
    assert len(result) == 3
    assert result[0][:2] == [10, 5]
    assert result[1][:2] == [11, 5]
    assert result[2][:2] == [14, 5]

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.

1 participant