Request #714: Apply indirect=True on particular argnames #908
Conversation
| valtype = indirect and "params" or "funcargs" | ||
| else: | ||
| if not isinstance(indirect, (tuple, list)): | ||
| valtypes = {arg: "params" for arg in argnames} |
There was a problem hiding this comment.
It seems dict comprehensions are invalid syntax in Python 2.6, which is why the checks are failing. You can probably do something like dict([(arg, "params") for arg in argnames]) instead (but I didn't test it 😉).
There was a problem hiding this comment.
Ah, nevermind - you were faster! 😆
|
Thanks for this @Elizaveta239! Could you please also add a note the the |
|
Hi @nicoddemus! Could you help me please? I added a test for bad 'indirect' parameter in order to increase code coverage. But this test isn't marked as 'passed', but marked as 'error'. How should I change it? |
|
Sure thing, can you post the failing test? Also, I think this feature deserves some documentation on it... could please add that too? 😄 |
|
@nicoddemus I said about 'test_parametrize_indirect_list_error' in metafunc.py. But now I figured out that the problem appeared because of my local changes. Sorry for disinformation! :) |
|
I guess it is ready now, right? 😄 @pfctdayelise, could you also take a look? |
|
Not yet, I'm going to update documentation soon :) |
| @@ -889,13 +892,22 @@ def parametrize(self, argnames, argvalues, indirect=False, ids=None, | |||
| if scope is None: | |||
| scope = "function" | |||
| scopenum = scopes.index(scope) | |||
| valtypes = dict.fromkeys(argnames, "funcargs") | |||
There was a problem hiding this comment.
dict.fromkeys, I didn't know that, that's cool :)
This section needs to be tidied up a bit. I think it would be simpler if you first deal with setting the right values in the valtypes dict, based on the value of indirect.
Then, secondly, go through that dict and check if we need to raise the "uses no fixture" or "fixture doesn't exist" exceptions.
Also, rather than checking if indirect is a tuple or list, it might be nicer to explicitly check for True and False, else assume it is a sequence and iterate over it.
|
The fixes after code review and docs are ready :) |
|
OK @Elizaveta239 I made some changes to your branch, but now I am not sure how to get them back to you! 😟 anyway they are in 40fa7b2 (merged up to date, and fixed merge conflict in CHANGELOG) and 36b86af (added test cases). In the test cases I added two which are failing but which should be passing. Do you want to see if you can get them passing? If not, I can do it, but you're very close to completing this :) |
|
@pfctdayelise Thank you! But I will not be able to do it during next two weeks :( If it's possible to wait until then -- I will fix these tests. |
|
@Elizaveta239 if it's ok with you I could fix them - it's pretty minor and I think it's better to get PRs in rather than have them lingering for ages. But if you really want to tackle them I can leave it for you :) |
… fixtures in test functions.
|
@pfctdayelise Hi! I'm online again :) I added a small fix for passing your new tests and fixed one old test. But authomatic checks have failed and I can't understand why. |
|
@Elizaveta239 can you add another commit or perhaps even rebase to master, it seems the checks refuse to run due to automated merging isn't working in something like the changelog |
# Conflicts: # AUTHORS
|
@pfctdayelise I merged master to my branch and now authomatic checks are passed. |
|
@RonnyPfannschmidt I merged master to my branch and it helped. Thank you! :) |
I wanted to ask that as well. 😄 |
|
@Elizaveta239 I think I checked out your master, but I either didn't fork it first, or the remotes are not set up correctly. So they're kind of floating around in the git ether somewhere! |
|
Is it possible to merge 36b86af into this? I don't mind even if it's done as a patch. |
|
@pfctdayelise just do a pull request against @Elizaveta239 's branch? |
Agree, then when @pfctdayelise merges here own branch this PR will be also merged automatically. 😊 |
The results of EuroPython 2015 sprint. See #714.