Skip to content

Refactor idmaker functions into class IdMaker #9547

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
merged 1 commit into from
Jan 27, 2022

Conversation

bluetech
Copy link
Member

ran: This is a commit cherry-picked from #9420 by @haxtibal with some tweaks by me. It is meant to accommodate the other changes in #9420 but I think it's nice regardless so picking it out already to reduce the size of #9420.


This commit only refactors, it does not change or add functionality yet. Public
API is retained. Reason or refactoring:

User provided parameter IDs (e.g. Metafunc.parametrize(ids=...)) had so far
only been used to calculate a unique test ID for each test invocation. That
test ID was a joined string where each parameter contributed some partial ID.

We're soon going to reuse functionality to generate parameter keys for
reorder_items and FixtureDef cache. We will be interested in the partial
IDs, and only if they originate from explicit user information. Refactoring
makes logic and data accessible for reuse, and increases cohesion in general.

This commit only refactors, it does not change or add functionality yet. Public
API is retained. Reason or refactoring:

User provided parameter IDs (e.g. Metafunc.parametrize(ids=...)) had so far
only been used to calculate a unique test ID for each test invocation. That
test ID was a joined string where each parameter contributed some partial ID.

We're soon going to reuse functionality to generate parameter keys for
reorder_items and FixtureDef cache. We will be interested in the partial
IDs, and only if they originate from explicit user information. Refactoring
makes logic and data accessible for reuse, and increases cohesion in general.
@bluetech bluetech merged commit 04bddfc into pytest-dev:main Jan 27, 2022
@nicoddemus
Copy link
Member

Should we backport this to 7.0.x to avoid conflicts in the future?

@bluetech
Copy link
Member Author

IMO, no. If there's ever a conflict that's too hard to resolve, we can backport it then. But the stable branch should be as stable as possible - there's always a risk of backporting bugs...

@The-Compiler
Copy link
Member

Agreed. I'd really like to avoid anything rather risky at this point, we've had way too much delay already. Maybe for 7.0.1 if we decide it's stable enough and we need it to make backporting easier, but I'd like to keep it away from 7.0.0 (also to make the diff between rc1 and final smaller)

@bluetech bluetech deleted the refactor-idmaker branch February 7, 2022 13:29
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.

3 participants