Skip to content

Commit

Permalink
Remove branch coverage and refactor coverage.py methods for accessing (
Browse files Browse the repository at this point in the history
…microsoft#24180)

Removing branch coverage from the payload as the initial way it was
being discovered / added was not documented / in the coverage.py API and
therefore is not guaranteed to be stable. Removing for now with plans to
add upstream and re-include in the extension here

coverage PR: microsoft#24118
  • Loading branch information
eleanorjboyd authored Sep 25, 2024
1 parent 30b7884 commit 710d3c8
Show file tree
Hide file tree
Showing 9 changed files with 26 additions and 92 deletions.
6 changes: 0 additions & 6 deletions python_files/tests/pytestadapter/test_coverage.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,3 @@ def test_simple_pytest_coverage():
assert focal_function_coverage.get("lines_missed") is not None
assert set(focal_function_coverage.get("lines_covered")) == {4, 5, 7, 9, 10, 11, 12, 13, 14, 17}
assert set(focal_function_coverage.get("lines_missed")) == {18, 19, 6}
assert (
focal_function_coverage.get("executed_branches") > 0
), "executed_branches are a number greater than 0."
assert (
focal_function_coverage.get("total_branches") > 0
), "total_branches are a number greater than 0."
6 changes: 0 additions & 6 deletions python_files/tests/unittestadapter/test_coverage.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,3 @@ def test_basic_coverage():
assert focal_function_coverage.get("lines_missed") is not None
assert set(focal_function_coverage.get("lines_covered")) == {4, 5, 7, 9, 10, 11, 12, 13, 14}
assert set(focal_function_coverage.get("lines_missed")) == {6}
assert (
focal_function_coverage.get("executed_branches") > 0
), "executed_branches are a number greater than 0."
assert (
focal_function_coverage.get("total_branches") > 0
), "total_branches are a number greater than 0."
31 changes: 13 additions & 18 deletions python_files/unittestadapter/execution.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import traceback
import unittest
from types import TracebackType
from typing import Dict, Iterator, List, Optional, Tuple, Type, Union
from typing import Dict, List, Optional, Set, Tuple, Type, Union

# Adds the scripts directory to the PATH as a workaround for enabling shell for test execution.
path_var_name = "PATH" if "PATH" in os.environ else "Path"
Expand Down Expand Up @@ -375,31 +375,26 @@ def send_run_data(raw_data, test_run_pipe):
)

if is_coverage_run:
from coverage.plugin import FileReporter
from coverage.report_core import get_analysis_to_report
from coverage.results import Analysis
import coverage

if not cov:
raise VSCodeUnittestError("Coverage is enabled but cov is not set")
cov.stop()
cov.save()
analysis_iterator: Iterator[Tuple[FileReporter, Analysis]] = get_analysis_to_report(
cov, None
)

cov.load()
file_set: Set[str] = cov.get_data().measured_files()
file_coverage_map: Dict[str, FileCoverageInfo] = {}
for fr, analysis in analysis_iterator:
file_str: str = fr.filename
executed_branches = analysis.numbers.n_executed_branches
total_branches = analysis.numbers.n_branches

for file in file_set:
analysis = cov.analysis2(file)
lines_executable = {int(line_no) for line_no in analysis[1]}
lines_missed = {int(line_no) for line_no in analysis[3]}
lines_covered = lines_executable - lines_missed
file_info: FileCoverageInfo = {
"lines_covered": list(analysis.executed), # set
"lines_missed": list(analysis.missing), # set
"executed_branches": executed_branches, # int
"total_branches": total_branches, # int
"lines_covered": list(lines_covered), # list of int
"lines_missed": list(lines_missed), # list of int
}
file_coverage_map[file_str] = file_info
file_coverage_map[file] = file_info

payload_cov: CoveragePayloadDict = CoveragePayloadDict(
coverage=True,
cwd=os.fspath(cwd),
Expand Down
2 changes: 0 additions & 2 deletions python_files/unittestadapter/pvsc_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,6 @@ class EOTPayloadDict(TypedDict):
class FileCoverageInfo(TypedDict):
lines_covered: List[int]
lines_missed: List[int]
executed_branches: int
total_branches: int


class CoveragePayloadDict(Dict):
Expand Down
44 changes: 10 additions & 34 deletions python_files/vscode_pytest/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
Any,
Dict,
Generator,
Iterator,
Literal,
TypedDict,
)
Expand Down Expand Up @@ -66,8 +65,6 @@ def __init__(self, message):
TEST_RUN_PIPE = os.getenv("TEST_RUN_PIPE")
SYMLINK_PATH = None

INCLUDE_BRANCHES = False


def pytest_load_initial_conftests(early_config, parser, args): # noqa: ARG001
global TEST_RUN_PIPE
Expand All @@ -84,10 +81,6 @@ def pytest_load_initial_conftests(early_config, parser, args): # noqa: ARG001
global IS_DISCOVERY
IS_DISCOVERY = True

if "--cov-branch" in args:
global INCLUDE_BRANCHES
INCLUDE_BRANCHES = True

# check if --rootdir is in the args
for arg in args:
if "--rootdir=" in arg:
Expand Down Expand Up @@ -366,8 +359,6 @@ def check_skipped_condition(item):
class FileCoverageInfo(TypedDict):
lines_covered: list[int]
lines_missed: list[int]
executed_branches: int
total_branches: int


def pytest_sessionfinish(session, exitstatus):
Expand Down Expand Up @@ -435,41 +426,26 @@ def pytest_sessionfinish(session, exitstatus):
)
# send end of transmission token

# send coverageee if enabled
# send coverage if enabled
is_coverage_run = os.environ.get("COVERAGE_ENABLED")
if is_coverage_run == "True":
# load the report and build the json result to return
import coverage
from coverage.report_core import get_analysis_to_report

if TYPE_CHECKING:
from coverage.plugin import FileReporter
from coverage.results import Analysis

cov = coverage.Coverage()
cov.load()
analysis_iterator: Iterator[tuple[FileReporter, Analysis]] = get_analysis_to_report(
cov, None
)

file_set: set[str] = cov.get_data().measured_files()
file_coverage_map: dict[str, FileCoverageInfo] = {}
for fr, analysis in analysis_iterator:
file_str: str = fr.filename
executed_branches = analysis.numbers.n_executed_branches
total_branches = analysis.numbers.n_branches
if not INCLUDE_BRANCHES:
print("coverage not run with branches")
# if covearge wasn't run with branches, set the total branches value to -1 to signal that it is not available
executed_branches = 0
total_branches = -1

for file in file_set:
analysis = cov.analysis2(file)
lines_executable = {int(line_no) for line_no in analysis[1]}
lines_missed = {int(line_no) for line_no in analysis[3]}
lines_covered = lines_executable - lines_missed
file_info: FileCoverageInfo = {
"lines_covered": list(analysis.executed), # set
"lines_missed": list(analysis.missing), # set
"executed_branches": executed_branches, # int
"total_branches": total_branches, # int
"lines_covered": list(lines_covered), # list of int
"lines_missed": list(lines_missed), # list of int
}
file_coverage_map[file_str] = file_info
file_coverage_map[file] = file_info

payload: CoveragePayloadDict = CoveragePayloadDict(
coverage=True,
Expand Down
3 changes: 1 addition & 2 deletions python_files/vscode_pytest/run_pytest_script.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,7 @@ def run_pytest(args):
coverage_enabled = True
break
if not coverage_enabled:
print("Coverage is enabled, adding branch coverage as an argument.")
args = [*args, "--cov=.", "--cov-branch"]
args = [*args, "--cov=."]

run_test_ids_pipe = os.environ.get("RUN_TEST_IDS_PIPE")
if run_test_ids_pipe:
Expand Down
18 changes: 2 additions & 16 deletions src/client/testing/testController/common/resultResolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,11 +133,6 @@ export class PythonResultResolver implements ITestResultResolver {
} else {
this._resolveExecution(payload as ExecutionTestPayload, runInstance);
}
if ('coverage' in payload) {
// coverage data is sent once per connection
traceVerbose('Coverage data received.');
this._resolveCoverage(payload as CoveragePayload, runInstance);
}
}

public _resolveCoverage(payload: CoveragePayload, runInstance: TestRun): void {
Expand All @@ -149,22 +144,13 @@ export class PythonResultResolver implements ITestResultResolver {
const fileCoverageMetrics = value;
const linesCovered = fileCoverageMetrics.lines_covered ? fileCoverageMetrics.lines_covered : []; // undefined if no lines covered
const linesMissed = fileCoverageMetrics.lines_missed ? fileCoverageMetrics.lines_missed : []; // undefined if no lines missed
const executedBranches = fileCoverageMetrics.executed_branches;
const totalBranches = fileCoverageMetrics.total_branches;

const lineCoverageCount = new TestCoverageCount(
linesCovered.length,
linesCovered.length + linesMissed.length,
);
const uri = Uri.file(fileNameStr);
let fileCoverage: FileCoverage;
if (totalBranches === -1) {
// branch coverage was not enabled and should not be displayed
fileCoverage = new FileCoverage(uri, lineCoverageCount);
} else {
const branchCoverageCount = new TestCoverageCount(executedBranches, totalBranches);
fileCoverage = new FileCoverage(uri, lineCoverageCount, branchCoverageCount);
}
const fileCoverage = new FileCoverage(uri, lineCoverageCount);
runInstance.addCoverage(fileCoverage);

// create detailed coverage array for each file (only line coverage on detailed, not branch)
Expand All @@ -189,7 +175,7 @@ export class PythonResultResolver implements ITestResultResolver {
detailedCoverageArray.push(statementCoverage);
}

this.detailedCoverageMap.set(fileNameStr, detailedCoverageArray);
this.detailedCoverageMap.set(uri.fsPath, detailedCoverageArray);
}
}

Expand Down
4 changes: 0 additions & 4 deletions src/client/testing/testController/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -279,10 +279,6 @@ export type FileCoverageMetrics = {
lines_covered: number[];
// eslint-disable-next-line camelcase
lines_missed: number[];
// eslint-disable-next-line camelcase
executed_branches: number;
// eslint-disable-next-line camelcase
total_branches: number;
};

export type ExecutionTestPayload = {
Expand Down
4 changes: 0 additions & 4 deletions src/test/testing/common/testingAdapter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -768,8 +768,6 @@ suite('End to End Tests: test adapters', () => {
// since only one test was run, the other test in the same file will have missed coverage lines
assert.strictEqual(simpleFileCov.lines_covered.length, 3, 'Expected 1 line to be covered in even.py');
assert.strictEqual(simpleFileCov.lines_missed.length, 1, 'Expected 3 lines to be missed in even.py');
assert.strictEqual(simpleFileCov.executed_branches, 1, 'Expected 3 lines to be missed in even.py');
assert.strictEqual(simpleFileCov.total_branches, 2, 'Expected 3 lines to be missed in even.py');
return Promise.resolve();
};

Expand Down Expand Up @@ -823,8 +821,6 @@ suite('End to End Tests: test adapters', () => {
// since only one test was run, the other test in the same file will have missed coverage lines
assert.strictEqual(simpleFileCov.lines_covered.length, 3, 'Expected 1 line to be covered in even.py');
assert.strictEqual(simpleFileCov.lines_missed.length, 1, 'Expected 3 lines to be missed in even.py');
assert.strictEqual(simpleFileCov.executed_branches, 1, 'Expected 3 lines to be missed in even.py');
assert.strictEqual(simpleFileCov.total_branches, 2, 'Expected 3 lines to be missed in even.py');

return Promise.resolve();
};
Expand Down

0 comments on commit 710d3c8

Please sign in to comment.