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.

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

Allow custom fixture names for fixtures #1444

Merged

Conversation

@novas0x2a
Copy link
Contributor

@novas0x2a novas0x2a commented Mar 9, 2016

When defining a fixture in the same module as where it is used, the
function argument shadows the fixture name, which a) annoys pylint and
b) can lead to bugs where you forget pull a fixture into a test method.

This allows one to define fixtures with a custom name override,
bypassing that problem.

I felt a little weird about thanking myself in the changelog.

@novas0x2a novas0x2a force-pushed the fixture-custom-name branch from c65bf43 to 4381248 Mar 9, 2016
@The-Compiler
Copy link
Member

@The-Compiler The-Compiler commented Mar 9, 2016

LGTM but I'd rather let someone else do the merge as I don't know the fixture code 😉

Thanks!

Loading

@RonnyPfannschmidt
Copy link
Member

@RonnyPfannschmidt RonnyPfannschmidt commented Mar 9, 2016

LGTM as well, but i'd like @hpk42 to be the one to merge this one

Loading

@nicoddemus
Copy link
Member

@nicoddemus nicoddemus commented Mar 9, 2016

👍 from me as well, I just think it is missing some mention in the docs (inclusive providing the rationale).

Loading

@novas0x2a
Copy link
Contributor Author

@novas0x2a novas0x2a commented Mar 9, 2016

Should I just add a new entry to the end of the fixture doc?

Loading

When defining a fixture in the same module as where it is used, the
function argument shadows the fixture name, which a) annoys pylint and
b) can lead to bugs where you forget to request a fixture into a test
method.

This allows one to define fixtures with a different name than the name
of the function, bypassing that problem.
@novas0x2a novas0x2a force-pushed the fixture-custom-name branch from 4381248 to 9577120 Mar 9, 2016
@novas0x2a
Copy link
Contributor Author

@novas0x2a novas0x2a commented Mar 9, 2016

Pushed a new version that updates the docstring (oops, forgot to do that originally). Hopefully this suffices for docs.

Loading

@nicoddemus
Copy link
Member

@nicoddemus nicoddemus commented Mar 10, 2016

I guess so. Thanks!

Loading

@novas0x2a
Copy link
Contributor Author

@novas0x2a novas0x2a commented Mar 14, 2016

anything i can do to make merging this easier?

Loading

@The-Compiler
Copy link
Member

@The-Compiler The-Compiler commented Mar 14, 2016

I guess if nobody else steps up I'll just merge this later today and be prepared to take the blame if something goes wrong (though I can't see what would) 😆

Loading

RonnyPfannschmidt added a commit that referenced this issue Mar 14, 2016
Allow custom fixture names for fixtures
@RonnyPfannschmidt RonnyPfannschmidt merged commit 0cacdef into pytest-dev:features Mar 14, 2016
2 checks passed
Loading
@nicoddemus
Copy link
Member

@nicoddemus nicoddemus commented Mar 14, 2016

👍

Loading

@pyup-bot pyup-bot mentioned this pull request Nov 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants