Currently, pants test timeouts for python are pretty coarse: • Timeouts apply at the file level. A file with a large number of tests may still be timed out, despite individual tests progressing • Timeouts for batched files are summed together, meaning a single stuck test can take ages to timeout Proposal: Add pytest-timeout, and implement timeouts per-case instead of per-file. 🧵

Interesting proposal! I would add a new option, not migrate the old one, just to keep things stable.

Hm, yea I suppose I could do this already; I could use the pytest args to set the global default per-case timeout. I was motivated to move this into Pants because the existing experience is pretty frustrating (a batch of 50 tests, each with a 1 min timeout, takes 50 mins to timeout 😮‍💨)

A friction point we run into is that we are defined at the Pants target level different kinds of tests (ex: unit, integration) -- which pytest knows nothing about -- but we would like the timeouts to be per case which Pants knows nothing about. thinking out loud: We could maybe set something like

PYTEST_TIMEOUT

or

--pytest-args

Hmm, does pytest-timeout move on to the next test when one times out, or does it fail early?

Yes, 50 mins is technically right. It just isn't intuitive or user friendly, and I think changing the default timeout system to per-test-case would be more reliable and a better overall UX. I'll experiment and see how it goes

An example would be really instructive, if you have a moment to describe one

Sure, let me try. Here are 3 example python test files, note the different argument to parameterize (and assume each test case should take ~1 second): `a_test.py`:

@pytest.mark.parametrize("test_input,expected", ARRAY_WITH_5_ITEMS)
def test_eval(test_input, expected):
    assert eval(test_input) == expected

`b_test.py`:

@pytest.mark.parametrize("test_input,expected", ARRAY_WITH_50_ITEMS)
def test_eval(test_input, expected):
    assert eval(test_input) == expected

`c_test.py`:

@pytest.mark.parametrize("test_input,expected", ARRAY_WITH_500_ITEMS)
def test_eval(test_input, expected):
    assert eval(test_input) == expected

With today's file-level timeout, there are 2 problems: 1. I can't just use

[test].timeout

to configure timeouts. I must use

BUILD

to override timeouts per-file (5, 50, and 500 seconds respectively) a. This becomes quite a burden with 1000s of test files b. *_I recognize that I'm assuming: test case duration is usually more consistent than number of tests in a file. This is my anecdotal experience, but maybe not always true_ 2. If 1 of 500 tests in

c_test.py

are stuck, I still have to wait 500 seconds for any response from pants a. This problem is massively worsened by batching. If 1 of 5 test in

a_test.py

are stuck and it happens to be batched with

c_test.py

, then I have to wait 505 seconds for any logs or indication, despite explicitly setting the timeout for

a_test.py

file to 5 seconds.

I see, but then what sensible value can you set the timeout to here? If c_test.py has a timeout shorter than 500 seconds, and each case takes 1 second, then pants will kill c_test.py before it can finish.

I meant that python test-level timeouts would be replaced entirely by pytest-timeout

Ah, so there would be no Pants-level timeout (or at least a very long one)

That’s one possibility, I hadn’t thought about it too much. Ideally the pants file timeout would be the sum of the case timeouts, but that gets into the slippery slope of pants test-case introspection