ionelmc / pytest-benchmark

py.test fixture for benchmarking code
BSD 2-Clause "Simplified" License
1.22k stars 115 forks source link

Undocumented behavior on `benchmark.weave` when patching class methods. #242

Open ckutlu opened 1 year ago

ckutlu commented 1 year ago

I just learned about pytest-benchmark, and it is such a nice little tool. Amazing work! I think I just hit a corner case in the experimental weave functionality, below is the description of it and a quick workaround for it.

Problem

I have a class Foo with a method internal decorated with classmethod. When I call benchmark.weave in a test case, the internal method of Foo is removed at the module level.

MWE

import time

import pytest

class Foo:
    def __init__(self, x: int):
        self.x = x

    @classmethod
    def internal(cls, x):
        time.sleep(x)

    def run(self):
        self.internal(self.x)

@pytest.mark.benchmark
def test_foo_internal(benchmark, x=1e-4):
    benchmark.weave(Foo.internal, lazy=True)
    foo = Foo(x)
    foo.run()

def test_foo_internal_exists():
    assert hasattr(Foo, "internal")

Output

================================================================================================================================== test session starts ===================================================================================================================================
platform linux -- Python 3.9.16, pytest-7.3.1, pluggy-1.0.0
benchmark: 4.0.0 (defaults: timer=time.perf_counter disable_gc=False min_rounds=5 min_time=0.000005 max_time=1.0 calibration_precision=10 warmup=False warmup_iterations=100000)
rootdir: /home/*****/
plugins: benchmark-4.0.0, xdist-3.3.1
collected 2 items

test_pytest_benchmark_weave_classmethod.py .F

======================================================================================================================================== FAILURES ========================================================================================================================================
________________________________________________________________________________________________________________________________ test_foo_internal_exists ________________________________________________________________________________________________________________________________

    def test_foo_internal_exists():
>       assert hasattr(Foo, "internal")
E       AssertionError: assert False
E        +  where False = hasattr(Foo, 'internal')

test_pytest_benchmark_weave_classmethod.py:36: AssertionError

-------------------------------------------------- benchmark: 1 tests --------------------------------------------------
Name (time in us)          Min       Max      Mean  StdDev    Median     IQR  Outliers  OPS (Kops/s)  Rounds  Iterations
------------------------------------------------------------------------------------------------------------------------
test_foo_internal     109.2280  349.1210  161.7128  7.7706  162.8630  5.3885    451;99        6.1838    6364           1
------------------------------------------------------------------------------------------------------------------------

Legend:
  Outliers: 1 Standard Deviation from Mean; 1.5 IQR (InterQuartile Range) from 1st Quartile and 3rd Quartile.
  OPS: Operations Per Second, computed as 1 / Mean
================================================================================================================================ short test summary info =================================================================================================================================
FAILED test_pytest_benchmark_weave_classmethod.py::test_foo_internal_exists - AssertionError: assert False
============================================================================================================================== 1 failed, 1 passed in 2.11s ===============================================================================================================================

When used together with pytest.mark.parametrize only the test case with the first parameter passes and subsequent ones fail.

Workaround

Seems like the underlying functionality from aspectlib covers class methods. Exploiting that, my workaround was supplying the 'methods' argument with the function name to be patched.

@pytest.mark.benchmark
def test_foo_internal(benchmark, x=1e-4):
    benchmark.weave(Foo, lazy=True, methods='internal')
    foo = Foo(x)
    foo.run()

This seems to cover my use case, but it doesn't look fitting to the API of pytest-benchmark. It'd be great if this use-case covered nicely. I'd be happy to try and implement it as a feature and a pull-request as well.

ckutlu commented 1 year ago

I also noticed that benchmark.stats "may be" unpopulated if lazy=True. This is not the case for the MWE above, but in the real codebase I had this issue which I couldn't reproduce with a simpler example yet.