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

Jump to bottom

parametrized: ids: support generator/iterator #6174

Merged
merged 1 commit into from
Nov 20, 2019
Merged

parametrized: ids: support generator/iterator #6174

merged 1 commit into from
Nov 20, 2019

Conversation

blueyed
Copy link
Contributor

@blueyed blueyed commented Nov 12, 2019

Fixes #759

Via blueyed#105.

@blueyed blueyed mentioned this pull request Nov 12, 2019
Closed
@blueyed
Copy link
Contributor Author

blueyed commented Nov 12, 2019 •
edited
Loading

Fails on py35 only due to random order (easy to fix / ignore, but IIRC there was an issue for that already maybe?):

E       Failed: nomatch: 'test_parametrize_iterator.py::test1[param0] PASSED'
E           and: '============================= test session starts =============================='
E           and: 'platform linux -- Python 3.5.6, pytest-5.2.3.dev233+gc8053b8d1, py-1.8.0, pluggy-0.13.0 -- /home/travis/build/pytest-dev/pytest/.tox/py35-xdist/bin/python'
E           and: 'cachedir: .pytest_cache'
E           and: "hypothesis profile 'default' -> database=DirectoryBasedExampleDatabase('/home/travis/build/pytest-dev/pytest/.hypothesis/examples')"
E           and: 'rootdir: /tmp/pytest-of-travis/pytest-0/popen-gw0/test_parametrize_iterator0'
E           and: 'plugins: hypothesis-4.44.2, forked-1.1.3, xdist-1.30.0'
E           and: 'collecting ... collected 6 items'
E           and: ''
E           and: 'test_parametrize_iterator.py::test1[param2] PASSED'
E           and: 'test_parametrize_iterator.py::test1[param3] PASSED'
E           and: 'test_parametrize_iterator.py::test2[param0] PASSED'
E           and: 'test_parametrize_iterator.py::test2[param1] PASSED'
E           and: 'test_parametrize_iterator.py::test_converted_to_str[1] PASSED'
E           and: 'test_parametrize_iterator.py::test_converted_to_str[2] PASSED'
E           and: ''
E           and: '============================== 6 passed in 0.04s ==============================='
E       remains unmatched: 'test_parametrize_iterator.py::test1[param0] PASSED'

(https://travis-ci.org/pytest-dev/pytest/jobs/611099976#L6281)

@blueyed
Copy link
Contributor Author

blueyed commented Nov 13, 2019

For reference I'm planning to add idsetfn or something similar afterwards (blueyed#104).

@blueyed blueyed force-pushed the ids-iter branch 2 times, most recently from b0d3ba6 to 7e59308 Compare November 13, 2019 12:45
@blueyed blueyed mentioned this pull request Nov 13, 2019
Closed
blueyed added a commit to blueyed/pytest that referenced this pull request Nov 13, 2019
@blueyed
cherry-pick 7e59308 (from pytest-dev#6174)
c202182
blueyed added a commit to blueyed/pytest that referenced this pull request Nov 13, 2019
@blueyed
cherry-pick 7e59308 (from pytest-dev#6174)
5985638
blueyed added a commit to blueyed/pytest that referenced this pull request Nov 13, 2019
@blueyed
cherry-pick 7e59308 (from pytest-dev#6174)
2d02935
@blueyed blueyed mentioned this pull request Nov 13, 2019
Open
4 tasks
blueyed added a commit to blueyed/pytest that referenced this pull request Nov 14, 2019
@blueyed
cherry-pick 7e59308 (from pytest-dev#6174)
8a3d7c7
@blueyed blueyed force-pushed the ids-iter branch 2 times, most recently from 10179ce to 167d5fb Compare November 19, 2019 01:15
blueyed
blueyed commented Nov 19, 2019
View reviewed changes
src/_pytest/python.py Outdated
else:
assert "ids" in kwargs
kwargs = kwargs.copy()
kwargs["ids"] = ids
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Having ids either as arg or kwarg is also not nice here.
Could be combined with _has_param_ids though at least.

src/_pytest/python.py Outdated
@@ -1016,6 +1042,7 @@ def parametrize(self, argnames, argvalues, indirect=False, ids=None, scope=None)
)
newcalls.append(newcallspec)
self._calls = newcalls
return ids
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not happy about having to return this here (for storing it in pytest_generate_tests then).

Copy link
Member

Choose a reason for hiding this comment

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

Solving the wrong problem at the wrong place feels like that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The main problem here is passing args and kwargs, but not the mark.
This could be done instead here also - but the function can be used by itself also (without a mark), so I wanted to keep it apart. But having an optional store_resolved_ids_on_mark kwarg might suffice then.

Returning the generated ids is not too bad also after all, but I've felt that it might be limiting in the future, and should therefore return the generated calls also etc.

src/_pytest/mark/structures.py Show resolved Hide resolved
RonnyPfannschmidt
RonnyPfannschmidt approved these changes Nov 19, 2019
View reviewed changes
src/_pytest/mark/structures.py
@@ -153,8 +161,19 @@ def combined_with(self, other):
combines by appending args and merging the mappings
"""
assert self.name == other.name

Copy link
Member

Choose a reason for hiding this comment

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

we might want to add a comment that this is intended to be a temporary hack to be resolved with object based marks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍
Will revisit before merging.

Thanks for the approval - I'm glad we could figure out something in the end.

@blueyed blueyed changed the title [RFC] parametrized: ids: support generator/iterator parametrized: ids: support generator/iterator Nov 20, 2019
@blueyed
parametrized: ids: support generator/iterator
2c941b5 
Fixes pytest-dev#759

- Adjust test_parametrized_ids_invalid_type, create list to convert tuples
  Ref: pytest-dev#1857 (comment)

- Changelog for int to str conversion
  Ref: pytest-dev#1857 (comment)
@blueyed blueyed merged commit ed012c8 into pytest-dev:features Nov 20, 2019
@blueyed blueyed deleted the ids-iter branch November 20, 2019 23:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RonnyPfannschmidt RonnyPfannschmidt RonnyPfannschmidt approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@blueyed @RonnyPfannschmidt