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

Fix stdout/stderr redirects #25

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
5 changes: 4 additions & 1 deletion src/pytest_codspeed/plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,10 @@ def pytest_runtest_protocol(item: pytest.Item, nextitem: pytest.Item | None):
assert plugin.lib is not None
runtest_call = pytest.CallInfo.from_call(
lambda: _run_with_instrumentation(
plugin.lib, item.nodeid, item.config, item.runtest
plugin.lib,
item.nodeid,
item.config,
lambda: ihook.pytest_runtest_call(item=item),
Copy link
Member

Choose a reason for hiding this comment

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

In the current state this might introduce variance in the code we measure since we'll also measure the pytest hook.
I think it would be better to do it the other way around: have pytest_runtest_call call _run_with_instrumentation

A way to do it might be to and override runtest but don't hesitate if you have another idea!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After digging some more I found we can use pytest_pyfunc_call instead of pytest_runtest_protocol (runtest invokes this hook, see https://github.com/pytest-dev/pytest/blob/5bb1363435a8cb3e2010505dbeb1e015c36beed6/src/_pytest/python.py#L1762-L1764)

I got the tests to pass locally but I may be missing something so let me know if you don't think this is the right thing to do

),
"call",
)
Expand Down