Skip to content

Commit be28121

Browse files
committed
fix: leading zeros can confuse human sorting. #1709
1 parent 1adda03 commit be28121

File tree

4 files changed

+29
-4
lines changed

4 files changed

+29
-4
lines changed

CHANGES.rst

+5
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@ Unreleased
2424
matching any of the lines, closing `issue 684`_. Thanks, `Jan Rusak,
2525
Maciej Kowalczyk and Joanna Ejzel <pull 1705_>`_.
2626

27+
- Fix: XML reports could fail with a TypeError if files had numeric components
28+
that were duplicates except for leading zeroes, like ``file1.py`` and
29+
``file001.py``. Fixes `issue 1709`_.
30+
2731
- The ``coverage annotate`` command used to announce that it would be removed
2832
in a future version. Enough people got in touch to say that they use it, so
2933
it will stay. Don't expect it to keep up with other new features though.
@@ -36,6 +40,7 @@ Unreleased
3640

3741
.. _issue 684: https://github.com/nedbat/coveragepy/issues/684
3842
.. _pull 1705: https://github.com/nedbat/coveragepy/pull/1705
43+
.. _issue 1709: https://github.com/nedbat/coveragepy/issues/1709
3944

4045

4146
.. scriv-start-here

coverage/misc.py

+7-3
Original file line numberDiff line numberDiff line change
@@ -338,9 +338,13 @@ def import_local_file(modname: str, modfile: Optional[str] = None) -> ModuleType
338338
return mod
339339

340340

341-
def _human_key(s: str) -> List[Union[str, int]]:
341+
def _human_key(s: str) -> Tuple[List[Union[str, int]], str]:
342342
"""Turn a string into a list of string and number chunks.
343-
"z23a" -> ["z", 23, "a"]
343+
344+
"z23a" -> (["z", 23, "a"], "z23a")
345+
346+
The original string is appended as a last value to ensure the
347+
key is unique enough so that "x1y" and "x001y" can be distinguished.
344348
"""
345349
def tryint(s: str) -> Union[str, int]:
346350
"""If `s` is a number, return an int, else `s` unchanged."""
@@ -349,7 +353,7 @@ def tryint(s: str) -> Union[str, int]:
349353
except ValueError:
350354
return s
351355

352-
return [tryint(c) for c in re.split(r"(\d+)", s)]
356+
return ([tryint(c) for c in re.split(r"(\d+)", s)], s)
353357

354358
def human_sorted(strings: Iterable[str]) -> List[str]:
355359
"""Sort the given iterable of strings the way that humans expect.

tests/test_misc.py

+3-1
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ def test_failure(self) -> None:
137137

138138

139139
HUMAN_DATA = [
140-
("z1 a2z a2a a3 a1", "a1 a2a a2z a3 z1"),
140+
("z1 a2z a01 a2a a3 a1", "a01 a1 a2a a2z a3 z1"),
141141
("a10 a9 a100 a1", "a1 a9 a10 a100"),
142142
("4.0 3.10-win 3.10-mac 3.9-mac 3.9-win", "3.9-mac 3.9-win 3.10-mac 3.10-win 4.0"),
143143
]
@@ -149,6 +149,8 @@ def test_human_sorted(words: str, ordered: str) -> None:
149149
@pytest.mark.parametrize("words, ordered", HUMAN_DATA)
150150
def test_human_sorted_items(words: str, ordered: str) -> None:
151151
keys = words.split()
152+
# Check that we never try to compare the values in the items
153+
human_sorted_items([(k, object()) for k in keys])
152154
items = [(k, 1) for k in keys] + [(k, 2) for k in keys]
153155
okeys = ordered.split()
154156
oitems = [(k, v) for k in okeys for v in [1, 2]]

tests/test_xml.py

+14
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,20 @@ def test_no_duplicate_packages(self) -> None:
350350
named_sub_package = dom.findall(".//package[@name='namespace.package.subpackage']")
351351
assert len(named_sub_package) == 1
352352

353+
def test_bug_1709(self) -> None:
354+
# https://github.com/nedbat/coveragepy/issues/1709
355+
self.make_file("main.py", "import x1y, x01y, x001y")
356+
self.make_file("x1y.py", "print('x1y')")
357+
self.make_file("x01y.py", "print('x01y')")
358+
self.make_file("x001y.py", "print('x001y')")
359+
360+
cov = coverage.Coverage()
361+
self.start_import_stop(cov, "main")
362+
assert self.stdout() == "x1y\nx01y\nx001y\n"
363+
# This used to raise:
364+
# TypeError: '<' not supported between instances of 'Element' and 'Element'
365+
cov.xml_report()
366+
353367

354368
def unbackslash(v: Any) -> Any:
355369
"""Find strings in `v`, and replace backslashes with slashes throughout."""

0 commit comments

Comments
 (0)