Flaky tests caused by mishandling of global PRNG state

[bukzor]

I've been running into this problem: https://paste.pound-python.org/show/qchV5aFbMkvs0uoYfArd/

This particular test has a @flaky decorator but no @given decorator. Despite the decorator, the problem was happening 100% reproducibly for me, and upon debugging, I found that the stdlib random singleton was in a deterministic state. Further, when running the test alone (-k "half and endpoint"), the test passed, and no longer had determinism.

This is caused by cross-test pollution. The @given decorator results in random.seed(0) running, so that the prior test causes the next test to be deterministic even though it wouldn't normally.

I'm not sure of the best fix, but this patch fixes the problem for me, and obviates the @flaky decorator too:

--- b/hypothesis-python/tests/common/debug.py
+++ a/tests/common/debug.py
@@ -73,6 +73,10 @@
         definition, condition=None,
         settings=None, random=None
 ):
+    if random is None:
+      from random import seed
+      seed(0)
+
     settings = Settings(
         settings,
         max_examples=10000,

[Zac-HD]

I think the proper solution will avoid causing this reseeding pollution in the first place. For example, a new context manager that can be used in core.py:

@contextlib.context_manager
def deterministic_context(seed=0):
    state = random.getstate()
    random.seed(seed)
    try:
        yield
    finally:
        random.setstate(state)

And then ban direct use of random.seed to avoid similar problems in future.

Note: this is equivalent to building the random_module() strategy into every use of @given, which seems like a user-interface win to me.

[bukzor]

@Zac-HD At that point, I think you'd be happier with an autouse pytest fixture. It's much more foolproof, and I feel sure you won't regret it.

In tests/conftest.py:

import pytest
import random

@pytest.fixture(autouse=True)
def deterministic_random():
    random.seed(0)

If you want to assert that seed isn't used, you can take it a bit farther:

import pytest
import random

@pytest.yield_fixture(autouse=True)
def deterministic_random():
    random.seed(0)
    seed = random.seed
    del random.seed
    yield
    random.seed = seed

[bukzor]

I think the proper solution will avoid causing this reseeding pollution in the first place.

To be clear, there's no reseeding involved in the problem above. The test does no seeding at all, but other tests in the module do, so it has differing behavior when run alone or along with all tests in the module.

[Zac-HD]

I mean that the other tests should not have the side-effect of changing the random state, which can be implemented via the decorator I suggested.

[Zac-HD]

Note: fixing this issue will remove the last reason to use the random_module strategy, which should therefore be turned into a no-op and deprecated.

Edit: if we integrate that into the internal runner mechanisms, which looks impractical.

[bukzor]

@Zac-HD Is there a ticket for that idea? How would I generate a pareto-random number in that world? Right now I can use st.randoms() and random.pareto().

[Zac-HD]

This issue is the ticket! I've assigned myself to make that clearer 😄

In that world you can use st.randoms() and methods on the instance it gives you (as now), or just use the global random - Hypothesis will pick a seed for each example and take care of resetting the global random state after each example.

[DRMacIver]

Note: fixing this issue will remove the last reason to use the random_module strategy, which should therefore be turned into a no-op and deprecated.

Wait, what? It might remove our last use of it, but the random_module strategy serves a perfectly valid (if somewhat niche) use case. Why would a change about how we test our code remove that?

[Zac-HD]

Sorry, looks like I need to actually write out my chain of logic here 😊

• The failure is caused by the way that every test with @given includes a random.seed(0) in core.py, but never resets the PRNG to its previous state. This massively degrades the quality of randomness in any test suite using Hypothesis, which is very bad. VERY VERY BAD. See e.g. test_uses_provided_seed passes without the @seed decorator #1174 for an example where this has bitten us 😭. We should fix the general problem rather than patching the tests to avoid it, especially because this impacts users too.

• The random_module() strategy is a special magic thing that doesn't give you a value (users: "what???"), but instead draws an integer seed and does the whole save/seed/resetting thing for you. Sadly, this only helps if you (a) know it exists, (b) know you need it for a particular test, and (c) remember to use it.

• So we need to do a thing where we save and reset the PRNG state in @given (and IIRC find, and for state machines - see Stateful testing doesn't explicitly seed global random #702) to avoid polluting the randomness of the rest of the test suite. If we do this, the damage is contained to each test where random_module() should have been used, but wasn't. 🎉

Before doing some deeper investigation I had hoped it would be practical to integrate the random_module() functionality into internals, but adding seeding to the mix makes it a lot harder.

[DRMacIver]

I agree that we should reset the random module state after given rather than letting random.seed(0) pollute the global state, and I'd be happy for suggestions for a better API for random_module (I agree it's weird), but the random_module strategy is there not to clean up state (which it doesn't - the random.seed(0) is called beforehand), but to allow people who are testing code which depends on the global random number generator to vary the seed between examples.

[Zac-HD]

I'll open a pull to reset state in a few days when life calms down a little 😄

We agree on the actual purpose of random_module being to vary the PRNG state between examples. However, it does reset the state in cleanup... right back the the random.seed(0) state that we set up earlier, which causes the original problem!

hypothesis/hypothesis-python/src/hypothesis/strategies.py

Lines 1002 to 1009 in d522674

	class RandomModule(SearchStrategy):
	def do_draw(self, data):
	data.can_reproduce_example_from_repr = False
	seed = data.draw(integers())
	state = random.getstate()
	random.seed(seed)
	cleanup(lambda: random.setstate(state))
	return RandomSeeder(seed)

[DRMacIver]

However, it does reset the state in cleanup...

Yes, sorry, that's what I meant by "the random.seed(0) is called beforehand". i.e. random_module does exactly as much state cleanup as without it effectively (less even! At least the state of the system after can vary a bit with normal @given)