Skip to content

repeat tests change to happen after -k #88

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

clemente0731
Copy link

@clemente0731 clemente0731 commented Jan 6, 2025

repeat tests change to happen after -k

Based on actual measurements under tests , the performance will be improved by at least 2x to 10x.

This change will optimize the performance of repeat cases, especially when there are a huge number of tests. If the "-k" option exists, only the cases matched by "-k" will be repeated.

after optimize --> 18 passed, 9 deselected in 0.01s

# pytest -vv -s -k test_factorial_of_large_number --count 18

plugins: asyncio-0.24.0, cov-6.0.0, repeat-0.9.3, html-4.1.1, metadata-3.1.1, anyio-4.4.0, timeout-2.3.1, typeguard-4.3.0
asyncio: mode=Mode.STRICT, default_loop_scope=None
collected 27 items / 9 deselected / 18 selected                                                                                                                                 

test_a.py::TestMathFunctions::test_factorial_of_large_number[1-18] PASSED
test_a.py::TestMathFunctions::test_factorial_of_large_number[2-18] PASSED
test_a.py::TestMathFunctions::test_factorial_of_large_number[3-18] PASSED
test_a.py::TestMathFunctions::test_factorial_of_large_number[4-18] PASSED
test_a.py::TestMathFunctions::test_factorial_of_large_number[5-18] PASSED
test_a.py::TestMathFunctions::test_factorial_of_large_number[6-18] PASSED
test_a.py::TestMathFunctions::test_factorial_of_large_number[7-18] PASSED
test_a.py::TestMathFunctions::test_factorial_of_large_number[8-18] PASSED
test_a.py::TestMathFunctions::test_factorial_of_large_number[9-18] PASSED
test_a.py::TestMathFunctions::test_factorial_of_large_number[10-18] PASSED
test_a.py::TestMathFunctions::test_factorial_of_large_number[11-18] PASSED
test_a.py::TestMathFunctions::test_factorial_of_large_number[12-18] PASSED
test_a.py::TestMathFunctions::test_factorial_of_large_number[13-18] PASSED
test_a.py::TestMathFunctions::test_factorial_of_large_number[14-18] PASSED
test_a.py::TestMathFunctions::test_factorial_of_large_number[15-18] PASSED
test_a.py::TestMathFunctions::test_factorial_of_large_number[16-18] PASSED
test_a.py::TestMathFunctions::test_factorial_of_large_number[17-18] PASSED
test_a.py::TestMathFunctions::test_factorial_of_large_number[18-18] PASSED

======================================================================= 18 passed, 9 deselected in 0.01s

before optimize --> 18 passed, 162 deselected in 0.02s

# pytest -vv -s -k test_factorial_of_large_number --count 18

**collected 180 items / 162 deselected** / 18 selected                                                                                                                              

test_a.py::TestMathFunctions::test_factorial_of_large_number[1-18] PASSED
test_a.py::TestMathFunctions::test_factorial_of_large_number[2-18] PASSED
test_a.py::TestMathFunctions::test_factorial_of_large_number[3-18] PASSED
test_a.py::TestMathFunctions::test_factorial_of_large_number[4-18] PASSED
test_a.py::TestMathFunctions::test_factorial_of_large_number[5-18] PASSED
test_a.py::TestMathFunctions::test_factorial_of_large_number[6-18] PASSED
test_a.py::TestMathFunctions::test_factorial_of_large_number[7-18] PASSED
test_a.py::TestMathFunctions::test_factorial_of_large_number[8-18] PASSED
test_a.py::TestMathFunctions::test_factorial_of_large_number[9-18] PASSED
test_a.py::TestMathFunctions::test_factorial_of_large_number[10-18] PASSED
test_a.py::TestMathFunctions::test_factorial_of_large_number[11-18] PASSED
test_a.py::TestMathFunctions::test_factorial_of_large_number[12-18] PASSED
test_a.py::TestMathFunctions::test_factorial_of_large_number[13-18] PASSED
test_a.py::TestMathFunctions::test_factorial_of_large_number[14-18] PASSED
test_a.py::TestMathFunctions::test_factorial_of_large_number[15-18] PASSED
test_a.py::TestMathFunctions::test_factorial_of_large_number[16-18] PASSED
test_a.py::TestMathFunctions::test_factorial_of_large_number[17-18] PASSED
test_a.py::TestMathFunctions::test_factorial_of_large_number[18-18] PASSED

====================================================================== 18 passed, 162 deselected in 0.02s =======================================================================

pytest -s --count 1 --collect-only -q

test_a.py::TestMathFunctions::test_add_positive
test_a.py::TestMathFunctions::test_add_negative
test_a.py::TestMathFunctions::test_divide
test_a.py::TestMathFunctions::test_divide_by_zero
test_a.py::TestMathFunctions::test_is_palindrome_true
test_a.py::TestMathFunctions::test_is_palindrome_false
test_a.py::TestMathFunctions::test_factorial_of_positive_number
test_a.py::TestMathFunctions::test_factorial_of_zero
test_a.py::TestMathFunctions::test_factorial_of_negative_number
test_a.py::TestMathFunctions::test_factorial_of_large_number

@clemente0731
Copy link
Author

@okken would you help to review this ? for issue-15

@clemente0731
Copy link
Author

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.

This needs some unitzests and currently completely misshandles complex keyword expressions

An expression with or would break the current simplified handling

We might want to upstream this optimization to pytest core for marker and keyword expressions to avoid generate tests altogether for deselected items

However the general case has some extra caveats about selection criteria potentially being part of the parameterset

For the case of keyword this is less of an issue with the caveats that markers from parametersets are part of the complete keyword set creating a slight potential for false negatives

@okken
Copy link
Contributor

okken commented Jan 9, 2025

I both agree with @RonnyPfannschmidt and think this is important to get to working.
So, @clemente0731, thanks for taking a look at this.

I'm wondering if, as a temporary hack until pytest core can deal with this properly, we can pre-apply keyword and marker deselection before applying multiplication in this plugin.

I don't know if it's possible, but it might be worth a try.
Looks like the deselection code in core starts here: https://github.com/pytest-dev/pytest/blob/535436f505461329e869361c3f792e17221877a6/src/_pytest/mark/__init__.py#L272, and it doesn't look too complicated. (fingers crossed).

@clemente0731, is that something you'd be willing to try, along with adding some more complex keyword and marker tests?
@RonnyPfannschmidt, is this duplication of deselection reasonable? crazy? worth a try anyway?

@RonnyPfannschmidt
Copy link
Member

Well its going to need some hacks so it depends

Off hand its unclear how fragile this will be

@clemente0731
Copy link
Author

I both agree with @RonnyPfannschmidt and think this is important to get to working.我既同意也认为这很重要,需要让它运作起来。 So, @clemente0731, thanks for taking a look at this.所以,感谢你花时间查看这个问题。

I'm wondering if, as a temporary hack until pytest core can deal with this properly, we can pre-apply keyword and marker deselection before applying multiplication in this plugin.我在想,作为一个临时的权宜之计,直到 pytest 核心可以妥善处理这个问题,我们是否可以在应用乘法操作之前预先应用关键字和标记的选择排除。

I don't know if it's possible, but it might be worth a try.我不确定是否可行,但值得一试。 Looks like the deselection code in core starts here: https://github.com/pytest-dev/pytest/blob/535436f505461329e869361c3f792e17221877a6/src/_pytest/mark/__init__.py#L272, and it doesn't look too complicated. (fingers crossed).看起来去选代码在核心是从这里开始的: https://github.com/pytest-dev/pytest/blob/535436f505461329e869361c3f792e17221877a6/src/_pytest/mark/__init__.py#L272,而且看起来并不太复杂。(希望如此)。

@clemente0731, is that something you'd be willing to try, along with adding some more complex keyword and marker tests?,这是你愿意尝试的,还包括一些更复杂的关键词和标记测试吗? @RonnyPfannschmidt, is this duplication of deselection reasonable? crazy? worth a try anyway?, 这个反选是否合理?疯狂?值得一试?

I'll give it a try and then update the results here. However, it might take a while as I'm quite busy with work this week. Ha~

@clemente0731
Copy link
Author

I both agree with @RonnyPfannschmidt and think this is important to get to working. So, @clemente0731, thanks for taking a look at this.

I'm wondering if, as a temporary hack until pytest core can deal with this properly, we can pre-apply keyword and marker deselection before applying multiplication in this plugin.

I don't know if it's possible, but it might be worth a try. Looks like the deselection code in core starts here: https://github.com/pytest-dev/pytest/blob/535436f505461329e869361c3f792e17221877a6/src/_pytest/mark/__init__.py#L272, and it doesn't look too complicated. (fingers crossed).

@clemente0731, is that something you'd be willing to try, along with adding some more complex keyword and marker tests? @RonnyPfannschmidt, is this duplication of deselection reasonable? crazy? worth a try anyway?

Already tried the pytest_collection_modifyitems method, but unfortunately, it seems that it cannot take effect before collection.

@RonnyPfannschmidt
Copy link
Member

The recommendation was to invoke the logic skip/deselect do in generate tests instead of modifyitems

@okken
Copy link
Contributor

okken commented Feb 16, 2025

I think this would be conceptually possible, but only for someone familiar with the AST and the parsing bits of deselect_by_keyword and deslect_by_mark.

Unless I'm mistaken, it looks like we'd have to re-implement something like deselect_by_keyword and deslect_by_mark to be used within pytest_genarate_tests in the plugin, to see if a test is likely to be selected or deselected, and then only repeat tests that are going to be selected.

The reimplementation seems necessary to apply the filtering from metafunc in pytest_generate_tests, as "items" is not available via metafunc, this will be difficult.

It seems like the right answer is for pytest to apply filtering via deselect_by_keyword and deslect_by_mark twice:

  • once before pytest_generate_tests
  • once after

A viable way to implement this within the pytest-repeat plugin eludes me.

I recommend closing this issue as "will not fix"

@okken okken closed this Feb 16, 2025
@okken okken reopened this Feb 16, 2025
@okken
Copy link
Contributor

okken commented Feb 16, 2025

Didn't intend to close it right now, I was hoping to get @RonnyPfannschmidt or others feedback first

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