pytest-dev / pytest-timeout

MIT License
206 stars 63 forks source link

Func Only Respects Environment Default #122

Closed ChrisWard42 closed 2 years ago

ChrisWard42 commented 2 years ago

A simple change to ensure that if func_only is not specified on a timeout marker, it can utilize the environment default. Presently when this function is called as part of _get_item_settings it returns False, so it never enters the following check to utilize the environment default:

if func_only is None:
        func_only = item.config._env_timeout_func_only

This change should not impact the usage in get_env_settings since that function converts it to a boolean via return Settings(timeout, method, func_only or False).

The motivation is that we were seeing tests fail in fixtures due to timeouts when the default in pytest.ini was set to timeout_func_only = true. The only present workaround is to explicitly specify the func_only argument to every instance of a timeout marker on individual tests.

I validated this change on the branch by running the following check before/after:

pytest.ini

timeout_method = signal
timeout = 5
timeout_func_only = true

test

import pytest
import time

class TestClass:
    @pytest.fixture(autouse=False, scope='function')
    def wait_a_bit(self):
        time.sleep(10.0)
        yield

    def test_pytest_timeout_func_only_no_marker(self, wait_a_bit):
        assert True

    @pytest.mark.timeout(3.0)
    def test_pytest_timeout_func_only_explicit_marker(self, wait_a_bit):
        assert True

The first test passes against both master and branch because it doesn't try to parse the marker. The second test fails against master and passes against this branch. Let me know if there's any other tests I should run.

flub commented 2 years ago

Hello, thanks for catching this! As you already have a manual test could you convert it into a test function in the tests? It would be great to have a regression tests against this.

jacobschaer commented 2 years ago

@ChrisWard42 https://github.com/ChrisWard42/pytest-timeout/pull/1

ChrisWard42 commented 2 years ago

@flub Definitely, I merged @jacobschaer commit into my branch (thanks Jacob :P) which should cover this case. Looks like the checks failed on a Windows build, though not in any of the parts of the file that were touched.

flub commented 2 years ago

thanks!