Skip to content
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.

Sign up for GitHub

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

Improve idmaker name selection in case of duplicate ids in parametrize #1474

Merged
merged 4 commits into from
Mar 23, 2016
Merged

Improve idmaker name selection in case of duplicate ids in parametrize #1474

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b8c15a0
improved idmaker name selection in case of duplicate ids
palaviv Mar 22, 2016
412042d
added entry to CHANGELOG
palaviv Mar 22, 2016
c66aedf
checking first there are duplciates ids before changing to unique names
palaviv Mar 23, 2016
3ffce6a
Added thanks and PR links in changlog
palaviv Mar 23, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@
* parametrize ids can accept None as specific test id. The
automatically generated id for that argument will be used.

*
* improved idmaker name selection in case of duplicate ids in
Copy link
Member

Choose a reason for hiding this comment

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

Please include a "thanks palaviv" and mention #1474. You might want to do the same for the item just above this one. 😉

parametrize.

*

Expand Down
12 changes: 9 additions & 3 deletions _pytest/python.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import types
import sys
import math
import collections

import py
import pytest
Expand Down Expand Up @@ -1152,9 +1153,14 @@ def _idvalset(idx, valset, argnames, idfn, ids):
def idmaker(argnames, argvalues, idfn=None, ids=None):
ids = [_idvalset(valindex, valset, argnames, idfn, ids)
for valindex, valset in enumerate(argvalues)]
if len(set(ids)) < len(ids):
# user may have provided a bad idfn which means the ids are not unique
ids = [str(i) + testid for i, testid in enumerate(ids)]
duplicates = [testid for testid in ids if ids.count(testid) > 1]
Copy link
Member

Choose a reason for hiding this comment

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

A faster way to check for duplicates:

has_duplicates = len(ids) != len(set(ids))
if has_duplicates:

While usually it won't matter, for a large list of test ids the difference might be significant:

$python -m timeit -s "ids = list(range(1000))" "[testid for testid in ids if ids.count(testid) > 1]"
100 loops, best of 3: 14.2 msec per loop

$python -m timeit -s "ids = list(range(1000))" "len(ids) != len(set(ids))"
10000 loops, best of 3: 22.2 usec per loop

If I increase the number of ids to 10000 I have to kill python after several seconds using the first snippet, while the second finishes in just 262 usec. 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At first i used:

duplicates =  [testid for testid, count in collections.Counter(ids).items() if count > 1]

which is more efficient but Counter is 2.7+.
I will change the code to first check if there are duplicates.

Copy link
Member

Choose a reason for hiding this comment

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

... but Counter is 2.7+.

I figured. 😉

Thanks! 😄

if duplicates:
# The ids are not unique
counters = collections.defaultdict(lambda: 0)
for index, testid in enumerate(ids):
if testid in duplicates:
ids[index] = testid + str(counters[testid])
counters[testid] += 1
return ids

def showfixtures(config):
Expand Down
17 changes: 8 additions & 9 deletions testing/python/metafunc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,9 +238,9 @@ def ids(val):
(20, KeyError()),
("three", [1, 2, 3]),
], idfn=ids)
assert result == ["0a-a",
"1a-a",
"2a-a",
assert result == ["a-a0",
"a-a1",
"a-a2",
]

@pytest.mark.issue351
Expand All @@ -267,10 +267,9 @@ def test_idmaker_with_ids(self):

def test_idmaker_with_ids_unique_names(self):
from _pytest.python import idmaker
result = idmaker(("a", "b"), [(1, 2),
(3, 4)],
ids=["a", "a"])
assert result == ["0a", "1a"]
result = idmaker(("a"), [1,2,3,4,5],
ids=["a", "a", "b", "c", "b"])
assert result == ["a0", "a1", "b0", "c", "b1"]

def test_addcall_and_parametrize(self):
def func(x, y): pass
Expand Down Expand Up @@ -834,8 +833,8 @@ def test_function(a, b):
result = testdir.runpytest("-v")
assert result.ret == 1
result.stdout.fnmatch_lines_random([
"*test_function*0a*PASSED",
"*test_function*1a*FAILED"
"*test_function*a0*PASSED",
"*test_function*a1*FAILED"
])

@pytest.mark.parametrize(("scope", "length"),
Expand Down