Resolve param indices using param values, not parameterset index by sadra-barikbin · Pull Request #11257 · pytest-dev/pytest

sadra-barikbin · 2023-07-28T23:40:09Z

To compute param indices smarter when user passes multiple parameter sets with duplicate values to parametrize in order to have a better reordering experience. Currently we have :

@pytest.fixture(scope='module')
def fixture1(request):
    pass

@pytest.fixture(scope='module')
def fixture2(request):
    pass

@pytest.mark.parametrize("fixture1, fixture2", [("a", 0), ("b", 1), ("a", 2)], indirect=True)
def test(fixture1, fixture2):
    pass

resulting in inefficient ordering i.e.

<Function test[a-0]>
<Function test[b-1]>
<Function test[a-2]>

It's a follow-up to #11220 .

RonnyPfannschmidt · 2023-07-29T10:45:23Z

@bluetech after some review of #519 and #2207 i believe it looks fair to consider the ordering more correct

i think we need a crosscheck of how we can observe interactions if people have multiple tests with different sets of parameters and their setup/teardown

bluetech · 2023-08-10T06:08:40Z

@sadra-barikbin Can you please rebase this on latest main?

…rmining-param-indices-in-metafunc-parametrize

src/_pytest/python.py

to use 'pytester.runpytest' instead of using testing/python/metafunc.py::TestMetafunc::Metafunc to see if coverage get fixed

…rmining-param-indices-in-metafunc-parametrize

…-duplicate-values-when-determining-param-indices-in-metafunc-parametrize' into Improvement-catch-duplicate-values-when-determining-param-indices-in-metafunc-parametrize

RonnyPfannschmidt · 2023-12-30T15:20:38Z

Apologies but I'll be delayed in reviewing this, I'm still down with a cold

It would have been nice to land this in 8.x

sadra-barikbin · 2023-12-30T18:52:48Z

Apologies but I'll be delayed in reviewing this, I'm still down with a cold

It would have been nice to land this in 8.x

It's alright. I hope you recover soon.

RonnyPfannschmidt

i think we have to document this better

from my pov we assign parameters that are used multiple times in a expanded/multiplied parameter set the index of the firs occurrence

this can be seen as approximation of a "multi dimensional" index from filtered cross-product

src/_pytest/python.py

…rmining-param-indices-in-metafunc-parametrize

sadra-barikbin · 2024-01-09T19:48:46Z

@bluetech , could you please review the PR too?

cyw233 · 2024-05-13T23:44:43Z

Hi @sadra-barikbin, do we have any plan to merge this PR, please? cc. @RonnyPfannschmidt

…rmining-param-indices-in-metafunc-parametrize

for more information, see https://pre-commit.ci

obestwalter · 2024-06-20T11:54:57Z

Just resolved a minor conflict. Let's see if we can get this merged as it is already approved.

obestwalter · 2024-06-20T14:44:07Z

Our shiny new bot also enforces changelog entries now, so this needs to be added before it can be merged - could you please add that @sadra-barikbin?

webknjaz · 2024-10-11T00:53:48Z

@sadra-barikbin there's another merge conflict to be resolved in addition to the need of having a changelog fragment.

sadra-barikbin · 2024-10-11T13:57:51Z

I resolved the conflict but this PR introduces is a breaking change with regard to how indices are assigned to the direct params of the test functions (hence one of the tests fail). Currently, when whole parametrizations of a function take place ,the indices of a direct param is assigned to its occurrences sequentially all at once:

pytest/src/_pytest/python.py

Lines 1452 to 1456 in f373974

    
           def _recompute_direct_params_indices(self) -> None: 
        
               for argname, param_type in self._params_directness.items(): 
        
                   if param_type == "direct": 
        
                       for i, callspec in enumerate(self._calls): 
        
                           callspec.indices[argname] = i

To be able to merge this PR, I should bring the resolve_values_indices_in_parametersets logic in the method above which introduces change to the whole reordering.

sadra-barikbin added 2 commits July 29, 2023 02:45

Apply comments, rebase and a few improvements

9791e1d

Do the improvement

ae5849f

sadra-barikbin mentioned this pull request Jul 22, 2023

Some possible improvements in fixtures module #11234

Open

sadra-barikbin mentioned this pull request Aug 1, 2023

Move fixtures.py::add_funcarg_pseudo_fixture_def to Metafunc.parametrize #11220

Merged

sadra-barikbin added 4 commits August 10, 2023 12:51

Merge branch 'main' into Improvement-catch-duplicate-values-when-dete…

da499bb

…rmining-param-indices-in-metafunc-parametrize

Merge branch 'main' into Improvement-catch-duplicate-values-when-dete…

9b10ae7

…rmining-param-indices-in-metafunc-parametrize

Add a tiny test

30d3231

Add a test

70ca754

sadra-barikbin commented Dec 17, 2023

View reviewed changes

src/_pytest/python.py Show resolved Hide resolved

sadra-barikbin requested a review from bluetech December 17, 2023 11:09

sadra-barikbin added 6 commits December 19, 2023 01:15

Change a test

752d23d

to use 'pytester.runpytest' instead of using testing/python/metafunc.py::TestMetafunc::Metafunc to see if coverage get fixed

Merge branch 'main' into Improvement-catch-duplicate-values-when-dete…

d33f65d

…rmining-param-indices-in-metafunc-parametrize

Merge branch 'main' into Improvement-catch-duplicate-values-when-dete…

af390f5

…rmining-param-indices-in-metafunc-parametrize

Merge branch 'main' into Improvement-catch-duplicate-values-when-dete…

82a4e0b

…rmining-param-indices-in-metafunc-parametrize

Remove a test

d5bf27c

Revert a tiny change

18b7147

sadra-barikbin force-pushed the Improvement-catch-duplicate-values-when-determining-param-indices-in-metafunc-parametrize branch from 054a0ae to 18b7147 Compare December 29, 2023 16:29

sadra-barikbin added 3 commits December 29, 2023 19:59

Merge branch 'main' into Improvement-catch-duplicate-values-when-dete…

2f33561

…rmining-param-indices-in-metafunc-parametrize

Remove a redundant test

49322ce

Merge remote-tracking branch 'refs/remotes/upstream/Improvement-catch…

ed6c38c

…-duplicate-values-when-determining-param-indices-in-metafunc-parametrize' into Improvement-catch-duplicate-values-when-determining-param-indices-in-metafunc-parametrize

sadra-barikbin requested a review from RonnyPfannschmidt December 29, 2023 19:31

RonnyPfannschmidt approved these changes Jan 5, 2024

View reviewed changes

src/_pytest/python.py Outdated Show resolved Hide resolved

sadra-barikbin added 4 commits January 7, 2024 23:21

Improve docstring and type annotation

8361ea4

Merge branch 'main' into Improvement-catch-duplicate-values-when-dete…

3a3697d

…rmining-param-indices-in-metafunc-parametrize

Fix a test

b5a7b1a

Merge branch 'main' into Improvement-catch-duplicate-values-when-dete…

909f695

…rmining-param-indices-in-metafunc-parametrize

RonnyPfannschmidt mentioned this pull request Mar 22, 2024

Parametrization with Variables Unexpectedly Changes Fixture Scope from Class-Level to Function-Level #12146

Open

sadra-barikbin mentioned this pull request Mar 23, 2024

pytest 8.0 sorting tests with multiple parameterization is broken #12008

Closed

vperumal mentioned this pull request May 8, 2024

ACL tests are taking a long time to run in master sonic-net/sonic-mgmt#12715

Closed

obestwalter and others added 2 commits June 20, 2024 13:53

Merge branch 'main' into Improvement-catch-duplicate-values-when-dete…

7efd12b

…rmining-param-indices-in-metafunc-parametrize

[pre-commit.ci] auto fixes from pre-commit.com hooks

f86aa30

for more information, see https://pre-commit.ci

Resolve conflict; Add changelog

23cba90

psf-chronographer bot added the bot:chronographer:provided (automation) changelog entry is part of PR label Oct 11, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Resolve param indices using param values, not parameterset index#11257

Resolve param indices using param values, not parameterset index#11257
sadra-barikbin wants to merge 22 commits intopytest-dev:mainfrom
sadra-barikbin:Improvement-catch-duplicate-values-when-determining-param-indices-in-metafunc-parametrize

sadra-barikbin commented Jul 28, 2023 •

edited

Loading

Uh oh!

RonnyPfannschmidt commented Jul 29, 2023

Uh oh!

bluetech commented Aug 10, 2023

Uh oh!

Uh oh!

RonnyPfannschmidt commented Dec 30, 2023

Uh oh!

sadra-barikbin commented Dec 30, 2023

Uh oh!

RonnyPfannschmidt left a comment

Uh oh!

Uh oh!

sadra-barikbin commented Jan 9, 2024

Uh oh!

cyw233 commented May 13, 2024

Uh oh!

obestwalter commented Jun 20, 2024 •

edited

Loading

Uh oh!

obestwalter commented Jun 20, 2024

Uh oh!

webknjaz commented Oct 11, 2024

Uh oh!

sadra-barikbin commented Oct 11, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

Conversation

sadra-barikbin commented Jul 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RonnyPfannschmidt commented Jul 29, 2023

Uh oh!

bluetech commented Aug 10, 2023

Uh oh!

Uh oh!

RonnyPfannschmidt commented Dec 30, 2023

Uh oh!

sadra-barikbin commented Dec 30, 2023

Uh oh!

RonnyPfannschmidt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sadra-barikbin commented Jan 9, 2024

Uh oh!

cyw233 commented May 13, 2024

Uh oh!

obestwalter commented Jun 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

obestwalter commented Jun 20, 2024

Uh oh!

webknjaz commented Oct 11, 2024

Uh oh!

sadra-barikbin commented Oct 11, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

sadra-barikbin commented Jul 28, 2023 •

edited

Loading

obestwalter commented Jun 20, 2024 •

edited

Loading