Home Original page

bpo-30317: Parameterize necessary timeouts in multiprocessing tests by applio · Pull Request #1722 · python/cpython

@applio

Parameterize necessary timeouts in selected multiprocessing tests to better accommodate very slow buildbots.

@applio

@mention-bot

zware

vstinner

"""
N = 5
defaultTimeout = 30.0 # XXX Slow Windows buildbots need generous timeout
default_timeout = 30.0 * TIMEOUT_MULTIPLIER

Choose a reason for hiding this comment

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

Why not just "timeout"?

# Particular buildbots can be exceptionally slow. Rather than raise timeouts
# (necessary in some tests) across the board to accomodate them, sensible
# timeout values may be scaled by this multiplier on such a buildbot.
TIMEOUT_MULTIPLIER = float(os.getenv("CONF_TIMEOUT_MULTIPLIER", default=1))

Choose a reason for hiding this comment

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

I would prefer to see this constant in test.support. I also suggest to rename it to TEST_TIMEOUT_MULTIPLIER or TEST_TIMEOUT_FACTOR.

p.map_async(f, [1, 2, 3], callback=results.extend)
deadline = time.time() + 60 # up to 60 s to report the results
wait_in_secs = 10 * TIMEOUT_MULTIPLIER # up to 10 s to report the results
deadline = time.time() + wait_in_secs

Choose a reason for hiding this comment

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

Please use time.monotonic() rather than time.time() to implement a timeout to behave correctly when the system clock is updated.

@taleinat

Ping, @applio. If you're not going to address this, perhaps unassign yourself?

@vstinner

I spent a lot of time with @pablogsal to adjust many timeouts in multiprocessing tests and all buildbots are now green (tests pass). The number of multiprocessing random failures dropped a lot. I'm not sure that we still need this feature. If @applio doesn't reply, I suggest to close/reject it.