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

Revert the unintended change in tests reordering from #11220 #12542

Conversation

sadra-barikbin
Copy link
Contributor

@sadra-barikbin sadra-barikbin commented Jun 27, 2024

In #11220, an unintended change in reordering was introduced by changing the way indices were assigned to direct params. This PR reverts that change and reduces #11220 changes to just refactors.

After this PR we could safely decide on the solutions discussed in #12008, i.e. #12082 or the one initially introduced in #11220 .

Fixes #12008

@bluetech @RonnyPfannschmidt

@sadra-barikbin sadra-barikbin changed the title Revert unintended change in tests reordering from #11220 Revert the unintended change in tests reordering from #11220 Jun 27, 2024
@sadra-barikbin sadra-barikbin requested a review from bluetech June 27, 2024 16:54
@sadra-barikbin sadra-barikbin force-pushed the fix-unnoticed-change-in-reordering-from-pr-11220 branch from 5d038d9 to 859979f Compare June 27, 2024 16:55
…nge-in-reordering-from-pr-11220' into fix-unnoticed-change-in-reordering-from-pr-11220
@psf-chronographer psf-chronographer bot added the bot:chronographer:provided (automation) changelog entry is part of PR label Jul 7, 2024
Copy link
Member

@RonnyPfannschmidt RonnyPfannschmidt left a comment

Choose a reason for hiding this comment

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

thanks for addressing this

@RonnyPfannschmidt
Copy link
Member

@bluetech please give it a quick check as well, else i'd like to get this in soon

@sadra-barikbin do we need to backport this?

@sadra-barikbin
Copy link
Contributor Author

@bluetech please give it a quick check as well, else i'd like to get this in soon

@sadra-barikbin do we need to backport this?

To 8.2.2 and its earlier versions back to 8.0.1 or just 8.2.2?

@The-Compiler
Copy link
Member

We usually only do patch releases for the latest stable branch, i.e. 8.2.

@nicoddemus
Copy link
Member

It would be nice to get @bluetech's review here too before we merge this.

@RonnyPfannschmidt
Copy link
Member

in case its not possible to get ran's feedback this by Sunday, lets merge, i believe its good to go and we shouldn't block for too long just because hes busy

src/_pytest/python.py Outdated Show resolved Hide resolved
@RonnyPfannschmidt RonnyPfannschmidt merged commit 6a3ac51 into pytest-dev:main Aug 11, 2024
29 checks passed
@RonnyPfannschmidt
Copy link
Member

sorry for the delay and thanks for the patience

@nicoddemus
Copy link
Member

nicoddemus commented Aug 13, 2024

Thanks @RonnyPfannschmidt.

Given this is marked as bugfix, should we backport it?

@RonnyPfannschmidt
Copy link
Member

As bluetech removed the back port label I believe we ought not, else he can revise

@nicoddemus
Copy link
Member

Ahh I missed that, sorry. We should not backport then. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot:chronographer:provided (automation) changelog entry is part of PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pytest 8.0 sorting tests with multiple parameterization is broken
5 participants