Skip to content

pytest_make_parametrize_id hook #1535

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.

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

Merged

Conversation

palaviv
Copy link
Contributor

@palaviv palaviv commented Apr 25, 2016

This PR is implementing @nicoddemus suggestion in issue #930.

@@ -156,6 +156,12 @@ def pytest_pyfunc_call(pyfuncitem):
def pytest_generate_tests(metafunc):
""" generate (multiple) parametrized calls to a test function."""

@hookspec(firstresult=True)
def pytest_make_parametrize_id(val):
Copy link
Member

Choose a reason for hiding this comment

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

I thought it would be useful to also receive the config object. Some plugin might add some custom options that might affect this.

@nicoddemus
Copy link
Member

Thanks @palaviv for taking the time to work on this!

@palaviv
Copy link
Contributor Author

palaviv commented Apr 26, 2016

I added the config object to the hook.

@nicoddemus
Copy link
Member

If nobody else wants to chime in, I will merge this tomorrow. 😁

@RonnyPfannschmidt
Copy link
Member

RonnyPfannschmidt commented Apr 27, 2016

since this is a new hook, can we mark it as "experimental" ?
i don't suspect much change at first glance, but we that's also why its a good chance to experiment with such a concept/tool

@nicoddemus
Copy link
Member

since this is a new hook, can we mark it as "experimental" ?

I'm not totally against that, but could you clarify exactly what you want "experimental" to mean here in this context? For example, we might remove arguments or the hook itself in the future?

I ask because hooks are forward compatible, and if in the future we realize we need some other argument, we can just add that and not break anything.

I think we should define exactly what we mean by "experimental", like what might happen in the future with that feature, and when a feature should be no longer considered experimental. Marking something as "experimental" usually discourages people from using a feature, specially if we don't exactly define what "experimental" means.

@RonnyPfannschmidt
Copy link
Member

i see, i better do a discussion on the ml then

also currently only hook receivers are forward compatible

@nicoddemus
Copy link
Member

OK, thanks!

Regarding this PR, are you OK for us to merge it? If your only concern is if we should mark it as experimental or not, we can always do it later.

only hook receivers are forward compatible

What do you mean by "hook receiver" exactly?

@RonnyPfannschmidt RonnyPfannschmidt merged commit 6cc56b4 into pytest-dev:features Apr 27, 2016
@RonnyPfannschmidt
Copy link
Member

receiver is the called hook function within a plugin
the caller is the function invoking the hook
an example of a hook that needs a new argument but cant be changed is the hook for deselected test items since deselecting modifyitems hooks are are supposed to invoke it

@nicoddemus
Copy link
Member

I now see what you mean, thanks! 😄

Just to make sure I'm not missing anything then, the new pytest_make_parametrize_id is forward compatible then right?

@RonnyPfannschmidt
Copy link
Member

yes

@nicoddemus
Copy link
Member

OK 👍

@coveralls
Copy link

Coverage Status

Coverage increased (+0.009%) to 92.476% when pulling 9733127 on palaviv:parametrize-test-ids-hook into 52babba on pytest-dev:features.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants