Skip to content

Commit 44f8880

Browse files
merge_coverage: canonicalize file paths so aliases collapse
When a workload runs the same source file under different path forms across steps — e.g. relative `pkg/foo.py` when invoked from cwd, and absolute `/.../pkg/foo.py` when imported as a package — the per-step coverage files record both spellings as separate keys. `merge_coverage` keyed by exact filename string, so the merged result listed the same physical file twice: one entry with the actual coverage, another empty. This roughly halved the headline percentage for workloads that mix CLI invocations and pytest-driven imports of the same package. Group entries by `Path(p).resolve()` and union the executed/missing lines and branches across aliases; use the shortest original spelling as the display key so output stays readable when one form is relative. Falls back to the original string on OSError (e.g. files not present on the merging host).
1 parent 615d8b3 commit 44f8880

2 files changed

Lines changed: 128 additions & 18 deletions

File tree

src/slipcover/slipcover.py

Lines changed: 64 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -256,8 +256,29 @@ def add_summaries(cov: dict) -> None:
256256
cov['summary'] = g_summary
257257

258258

259+
def _canonical_path(p: str) -> str:
260+
"""Resolve a path key to a canonical form for cross-step equivalence.
261+
262+
The same physical file can be recorded under different path spellings
263+
across steps of a workload — most commonly a cwd-relative form when one
264+
step runs a script directly and an absolute editable-install form when
265+
another step imports it as a package. Without canonicalization the merge
266+
treats them as separate files and the headline percentage is halved.
267+
"""
268+
try:
269+
return str(Path(p).resolve())
270+
except OSError:
271+
return p
272+
273+
259274
def merge_coverage(a: dict, b: dict) -> dict:
260-
"""Merges coverage result 'b' into 'a'."""
275+
"""Merges coverage result 'b' into 'a'.
276+
277+
File entries are grouped by canonical path so that aliases of the same
278+
physical file (relative vs absolute, symlinked, etc.) collapse into a
279+
single merged entry. The shortest original spelling is used as the
280+
display key for each group.
281+
"""
261282

262283
if a.get('meta', {}).get('software', None) != 'slipcover':
263284
raise SlipcoverError('Cannot merge coverage: only SlipCover format supported.')
@@ -273,28 +294,53 @@ def merge_coverage(a: dict, b: dict) -> dict:
273294
a_files = a['files']
274295
b_files = b['files']
275296

276-
def both(f, field):
277-
return (a_files[f][field] if f in a_files else []) + b_files[f][field]
297+
# Group aliases by canonical path.
298+
groups: dict = defaultdict(lambda: {'a': [], 'b': []})
299+
for k in a_files:
300+
groups[_canonical_path(k)]['a'].append(k)
301+
for k in b_files:
302+
groups[_canonical_path(k)]['b'].append(k)
303+
304+
new_files: dict = {}
305+
for aliases in groups.values():
306+
executed_lines: set = set()
307+
missing_lines: set = set()
308+
executed_branches: set = set()
309+
missing_branches: set = set()
310+
311+
for k in aliases['a']:
312+
entry = a_files[k]
313+
executed_lines.update(entry.get('executed_lines', ()))
314+
missing_lines.update(entry.get('missing_lines', ()))
315+
if branch_coverage:
316+
executed_branches.update(tuple(br) for br in entry.get('executed_branches', ()))
317+
missing_branches.update(tuple(br) for br in entry.get('missing_branches', ()))
318+
for k in aliases['b']:
319+
entry = b_files[k]
320+
executed_lines.update(entry.get('executed_lines', ()))
321+
missing_lines.update(entry.get('missing_lines', ()))
322+
if branch_coverage:
323+
executed_branches.update(tuple(br) for br in entry.get('executed_branches', ()))
324+
missing_branches.update(tuple(br) for br in entry.get('missing_branches', ()))
278325

279-
for f in b_files:
280-
executed_lines = set(both(f, 'executed_lines'))
281-
missing_lines = set(both(f, 'missing_lines'))
282326
missing_lines -= executed_lines
283-
update = {
327+
missing_branches -= executed_branches
328+
329+
# Prefer the shortest original spelling as the display key (typically
330+
# the cwd-relative form when both relative and absolute are present).
331+
display = min(aliases['a'] + aliases['b'], key=lambda s: (len(s), s))
332+
333+
update: dict = {
284334
'executed_lines': sorted(executed_lines),
285-
'missing_lines': sorted(missing_lines)
335+
'missing_lines': sorted(missing_lines),
286336
}
287-
288337
if branch_coverage:
289-
executed_branches = set(tuple(br) for br in both(f, 'executed_branches'))
290-
missing_branches = set(tuple(br) for br in both(f, 'missing_branches'))
291-
missing_branches -= executed_branches
292-
update.update({
293-
'executed_branches': sorted(list(br) for br in executed_branches),
294-
'missing_branches': sorted(list(br) for br in missing_branches)
295-
})
296-
297-
a_files[f] = update
338+
update['executed_branches'] = sorted(list(br) for br in executed_branches)
339+
update['missing_branches'] = sorted(list(br) for br in missing_branches)
340+
new_files[display] = update
341+
342+
a_files.clear()
343+
a_files.update(new_files)
298344

299345
add_summaries(a)
300346
return a

tests/test_coverage.py

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -880,6 +880,70 @@ def test_merge_coverage(tmp_path, monkeypatch, do_branch):
880880
check_summaries(a)
881881

882882

883+
@pytest.mark.parametrize("do_branch", [True, False])
884+
def test_merge_coverage_canonicalizes_paths(tmp_path, monkeypatch, do_branch):
885+
"""When the same physical file is recorded under two different path
886+
spellings (e.g. relative + absolute, or via a symlink), merge_coverage
887+
should canonicalize the paths and collapse the entries into one.
888+
889+
This happens in practice when one workload step records files via a
890+
cwd-relative path (e.g. running `python -m pkg pkg/`) while another step
891+
imports the same files as a package, causing Python's import machinery
892+
to expose them via their editable-install absolute path.
893+
"""
894+
monkeypatch.chdir(tmp_path)
895+
896+
(tmp_path / "t.py").write_text("""\
897+
import sys
898+
if len(sys.argv) < 2: # 2
899+
print("A branch")
900+
else:
901+
print("B branch") # 5
902+
""")
903+
904+
subprocess.run([sys.executable, '-m', 'slipcover'] +
905+
(['--branch'] if do_branch else []) +
906+
['--json', '--out', tmp_path / "a.json", "t.py"], check=True)
907+
subprocess.run([sys.executable, '-m', 'slipcover'] +
908+
(['--branch'] if do_branch else []) +
909+
['--json', '--out', tmp_path / "b.json", "t.py", "X"], check=True)
910+
911+
with (tmp_path / "a.json").open() as f:
912+
a = json.load(f)
913+
with (tmp_path / "b.json").open() as f:
914+
b = json.load(f)
915+
916+
# Re-key b's entry to the absolute path that the same file resolves to.
917+
abs_path = str((tmp_path / "t.py").resolve())
918+
assert abs_path != "t.py"
919+
b['files'][abs_path] = b['files'].pop('t.py')
920+
921+
assert 't.py' in a['files']
922+
assert abs_path in b['files']
923+
924+
sc.merge_coverage(a, b)
925+
926+
# Aliases collapse into a single entry.
927+
t_keys = [k for k in a['files'] if k.endswith('t.py')]
928+
assert len(t_keys) == 1, f"expected one entry for t.py, got: {t_keys}"
929+
930+
# The shorter (relative) display form wins when both forms exist.
931+
assert t_keys[0] == 't.py'
932+
933+
# Coverage from both runs is unioned.
934+
# Run a (no extra arg) executes lines 1,2,3; run b (with arg) executes 1,2,5.
935+
assert {1, 2, 3, 5} <= set(a['files']['t.py']['executed_lines'])
936+
assert [] == a['files']['t.py']['missing_lines']
937+
938+
if do_branch:
939+
# Both branches of the `if` at line 2 should be covered.
940+
assert [2, 3] in a['files']['t.py']['executed_branches']
941+
assert [2, 5] in a['files']['t.py']['executed_branches']
942+
assert [] == a['files']['t.py']['missing_branches']
943+
944+
check_summaries(a)
945+
946+
883947
@pytest.fixture
884948
def cov_merge_fixture(tmp_path, monkeypatch):
885949
monkeypatch.chdir(tmp_path)

0 commit comments

Comments
 (0)