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.

Sign up for GitHub

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

Jump to bottom

v24.0 StepLogger may be started twice which results in errors. #1468

Open
jremmet opened this issue Aug 13, 2024 · 4 comments · May be fixed by #1474
Open

v24.0 StepLogger may be started twice which results in errors. #1468

jremmet opened this issue Aug 13, 2024 · 4 comments · May be fixed by #1474
Milestone
v24.0.1

Comments

@jremmet
Copy link
Contributor

jremmet commented Aug 13, 2024

We use pytester to test our hooks together with labgrid.
Since v24.0 we run into errors because StepLogger.start is called again by pytester via pytest_configure hook.

minimal example:
test_mini.py

import pytest

pytest_plugins = ["pytester"]

DUMMY_TEST = """
import pytest

def test_success():
    pass
"""

def test_pytester(pytester):
    """Test pytester."""
    pytester.makepyfile(DUMMY_TEST)
    result = pytester.runpytest()
    assert result.ret == pytest.ExitCode.OK

==================================================================== test session starts ====================================================================
platform linux -- Python 3.10.12, pytest-8.3.2, pluggy-1.5.0
rootdir: /tmp/labgtid
plugins: labgrid-24.0
collected 1 item                                                                                                                                            

test_mini.py F                                                                                                                                        [100%]

========================================================================= FAILURES ==========================================================================
_______________________________________________________________________ test_pytester _______________________________________________________________________

pytester = <Pytester PosixPath('/tmp/pytest-of-jremmet/pytest-0/test_pytester0')>

    def test_pytester(pytester):
        """Test pytester."""
        pytester.makepyfile(DUMMY_TEST)
        result = pytester.runpytest()
>       assert result.ret == pytest.ExitCode.OK
E       AssertionError: assert <ExitCode.INTERNAL_ERROR: 3> == <ExitCode.OK: 0>
E        +  where <ExitCode.INTERNAL_ERROR: 3> = <RunResult ret=ExitCode.INTERNAL_ERROR len(stdout.lines)=19 len(stderr.lines)=0 duration=0.02s>.ret
E        +  and   <ExitCode.OK: 0> = <enum 'ExitCode'>.OK
E        +    where <enum 'ExitCode'> = pytest.ExitCode

/tmp/labgtid/test_mini.py:16: AssertionError
------------------------------------------------------------------- Captured stdout call --------------------------------------------------------------------
INTERNALERROR> Traceback (most recent call last):
INTERNALERROR>   File "/tmp/labgtid/venv/lib/python3.10/site-packages/_pytest/main.py", line 279, in wrap_session
INTERNALERROR>     config._do_configure()
INTERNALERROR>   File "/tmp/labgtid/venv/lib/python3.10/site-packages/_pytest/config/__init__.py", line 1118, in _do_configure
INTERNALERROR>     self.hook.pytest_configure.call_historic(kwargs=dict(config=self))
INTERNALERROR>   File "/tmp/labgtid/venv/lib/python3.10/site-packages/pluggy/_hooks.py", line 535, in call_historic
INTERNALERROR>     res = self._hookexec(self.name, self._hookimpls.copy(), kwargs, False)
INTERNALERROR>   File "/tmp/labgtid/venv/lib/python3.10/site-packages/pluggy/_manager.py", line 120, in _hookexec
INTERNALERROR>     return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
INTERNALERROR>   File "/tmp/labgtid/venv/lib/python3.10/site-packages/pluggy/_callers.py", line 139, in _multicall
INTERNALERROR>     raise exception.with_traceback(exception.__traceback__)
INTERNALERROR>   File "/tmp/labgtid/venv/lib/python3.10/site-packages/pluggy/_callers.py", line 103, in _multicall
INTERNALERROR>     res = hook_impl.function(*args)
INTERNALERROR>   File "/tmp/labgtid/venv/lib/python3.10/site-packages/labgrid/pytestplugin/hooks.py", line 65, in pytest_configure
INTERNALERROR>     StepLogger.start()
INTERNALERROR>   File "/tmp/labgtid/venv/lib/python3.10/site-packages/labgrid/logging.py", line 147, in start
INTERNALERROR>     assert not cls._started
INTERNALERROR> AssertionError: assert not True
INTERNALERROR>  +  where True = <class 'labgrid.logging.StepLogger'>._started
================================================================== short test summary info ==================================================================
FAILED test_mini.py::test_pytester - AssertionError: assert <ExitCode.INTERNAL_ERROR: 3> == <ExitCode.OK: 0>
===================================================================== 1 failed in 0.05s =====================================================================

Is it possible to replace the asserion by a if clause here:

labgrid/labgrid/logging.py

Line 147 in 7f2c8fc

assert not cls._started

Or add a "is-already-running" check here:

labgrid/labgrid/pytestplugin/hooks.py

Line 65 in 7f2c8fc

StepLogger.start()

?
@Emantor
Copy link
Member

Emantor commented Aug 13, 2024

Thats unfortunate. I guess allowing multiple starts for the StepLogger should not be too bad, so I think we should handle that in the the Steplogger class, if @jluebbe and @Bastian-Krause agree.

@Emantor Emantor added this to the v24.0.1 milestone Aug 13, 2024
@jremmet jremmet linked a pull request Aug 13, 2024 that will close this issue
Open
7 tasks
@jremmet
Copy link
Contributor Author

jremmet commented Aug 13, 2024

Thats unfortunate. I guess allowing multiple starts for the StepLogger should not be too bad, so I think we should handle that in the the Steplogger class, if @jluebbe and @Bastian-Krause agree.

I added a PR which handle it in the pytest hook. If we just ignore multiple calls, we will run stop to early.
Alternatively we need to count the starts and stops, which seams fragile if called directly.

@Bastian-Krause
Copy link
Member

Thats unfortunate. I guess allowing multiple starts for the StepLogger should not be too bad, so I think we should handle that in the the Steplogger class, if @jluebbe and @Bastian-Krause agree.

I don't know the implications of that. As long as you say it's safe, I have no objections.

@jremmet
Copy link
Contributor Author

jremmet commented Nov 7, 2024

gently ping

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v24.0.1
Development

Successfully merging a pull request may close this issue.

labgrid: pytestplugin: hooks: fix multiple start stop for StepLogger jremmet/labgrid
3 participants
@Emantor @Bastian-Krause @jremmet