[tests] Make random seed logged and settable #15963

jnewbery commented at 5:19 PM on May 6, 2019: member

This allows tests which use randomness to be reproducibly run on failure.

jnewbery force-pushed on May 6, 2019

in test/functional/test_framework/test_framework.py:134 in 3105cf4f1c outdated

 129 | @@ -129,6 +130,8 @@ def main(self):
 130 |                              help="use bitcoin-cli instead of RPC for all commands")
 131 |          parser.add_argument("--perf", dest="perf", default=False, action="store_true",
 132 |                              help="profile running nodes with perf for the duration of the test")
 133 | +        parser.add_argument("--randseed", action="store",
 134 | +                            help="set a random seed for deterministically reproducing a previous test run")

MarcoFalke commented at 5:24 PM on May 6, 2019:

Should this be of type int?

jnewbery commented at 5:32 PM on May 6, 2019:

thanks. Updated

MarcoFalke approved

jnewbery force-pushed on May 6, 2019

promag commented at 5:58 PM on May 6, 2019: member

utACK e08e4c5. Does it make sense to add the same parameter to test/functional/test_runner.py?

in test/functional/test_framework/test_framework.py:167 in e08e4c5831 outdated

 160 | @@ -158,6 +161,21 @@ def main(self):
 161 |              self.options.tmpdir = tempfile.mkdtemp(prefix=TMPDIR_PREFIX)
 162 |          self._start_logging()
 163 |  
 164 | +        # Seed the PRNG.  Note that testruns are only reproducible if only a
 165 | +        # single thread accesses the PRNG (see
 166 | +        # https://docs.python.org/3/library/random.html#notes-on-reproducibility).
 167 | +        # The network thread shouldn't access random. If we need to change the

jonatack commented at 6:09 PM on May 6, 2019:

Alternatively:

        # Seed the PRNG. Note that testruns are reproducible if and only if
        # a single thread accesses the PRNG. For more information, see
        # https://docs.python.org/3/library/random.html#notes-on-reproducibility.

jnewbery commented at 1:57 PM on May 7, 2019:

Thanks. That's better. I've updated the comment.

in test/functional/test_framework/test_framework.py:133 in e08e4c5831 outdated

 129 | @@ -129,6 +130,8 @@ def main(self):
 130 |                              help="use bitcoin-cli instead of RPC for all commands")
 131 |          parser.add_argument("--perf", dest="perf", default=False, action="store_true",
 132 |                              help="profile running nodes with perf for the duration of the test")
 133 | +        parser.add_argument("--randseed", action="store", type=int,

MarcoFalke commented at 6:12 PM on May 6, 2019:

        parser.add_argument("--randseed", type=int,

syle-nit: No need to mention the default when it isn't done anywhere else in our code.

jonatack commented at 8:04 PM on May 6, 2019:

Perhaps just --seed for brevity

jnewbery commented at 1:57 PM on May 7, 2019:

removed

jnewbery commented at 1:58 PM on May 7, 2019:

I've gone the other way and changed this to randomseed. There's already a portseed argument, so more explicit is good (and I expect the randomseed argument to be used very infrequently so a few extra characters isn't an issue).

in test/functional/test_framework/test_framework.py:173 in e08e4c5831 outdated

 169 | +        # random.Random object.
 170 | +        if self.options.randseed is not None:
 171 | +            self.log.debug("Using user supplied random seed {}".format(self.options.randseed))
 172 | +            seed = self.options.randseed
 173 | +        else:
 174 | +            seed = random.randrange(sys.maxsize)

jonatack commented at 6:20 PM on May 6, 2019:

Alternatively, to use self.options.randseed only once, avoid a not conditional, and begin the conditional with the most frequent case:

        seed = self.options.randseed

        if seed is None:
            seed = random.randrange(sys.maxsize)
        else:
            self.log.debug("Using user supplied random seed {}".format(seed))

Feel free to ignore. FWIW, inline assignment in the conditional e.g. if seed := self.options.randseed is None: is coming to Python 3.8, IIUC.

jnewbery commented at 1:57 PM on May 7, 2019:

Looks good. I've taken your version

jonatack commented at 6:24 PM on May 6, 2019: member

tACK e08e4c5831aa68978844934a3e728a14d1ef6ae3

in test/functional/test_framework/test_framework.py:164 in e08e4c5831 outdated

 160 | @@ -158,6 +161,21 @@ def main(self):
 161 |              self.options.tmpdir = tempfile.mkdtemp(prefix=TMPDIR_PREFIX)
 162 |          self._start_logging()
 163 |  
 164 | +        # Seed the PRNG.  Note that testruns are only reproducible if only a

jonatack commented at 6:35 PM on May 6, 2019:

nit: git grepping shows "test run" is the usual spelling in this repo

jnewbery commented at 1:57 PM on May 7, 2019:

changed to 'test run'

DrahtBot added the label Tests on May 6, 2019

jnewbery commented at 1:59 PM on May 7, 2019: member

Does it make sense to add the same parameter to test/functional/test_runner.py?

The test_runner doesn't use any randomness.

jnewbery force-pushed on May 7, 2019

MarcoFalke commented at 2:04 PM on May 7, 2019: member

utACK 05f4713a315d53c807db814fb00c997ded4878c4

<details><summary>Show signature and timestamp</summary>

Signature:

Timestamp of file with hash c3482251ec46d8166fb2cddd2612cf65296d2b21c068d1881b716589f92e6f59 -

</details>

practicalswift commented at 2:13 PM on May 7, 2019: contributor

utACK 05f4713a315d53c807db814fb00c997ded4878c4

Nice!

in test/functional/test_framework/test_framework.py:175 in 05f4713a31 outdated

 170 | +        seed = self.options.randomseed
 171 | +
 172 | +        if self.options.randomseed is None:
 173 | +            seed = random.randrange(sys.maxsize)
 174 | +        else:
 175 | +            self.log.debug("User supplied random seed {}".format(self.options.randomseed))

jonatack commented at 6:52 PM on May 7, 2019:

self.options.randomseed can be simply seed in lines 172 and 175.

jonatack commented at 7:05 PM on May 7, 2019:

Just noted the log message change. Agreed, either "Using user-supplied" or "User supplied" may be better than "Using user supplied".

MarcoFalke commented at 7:06 PM on May 7, 2019:

Or maybe the local seed should be removed and the seed should be stored in the options member? That way it could be accessed by tests to (re-seed) to the inital seed?

jonatack commented at 7:20 PM on May 7, 2019:

Interesting option as well, if re-seeding would be useful.

jnewbery commented at 3:33 PM on May 9, 2019:

oops. Good spot @jonatack - fixed!

jonatack commented at 7:00 PM on May 7, 2019: member

Thanks @jnewbery. Good catch with portseed and randomseed. Re-ACK 05f4713a315d53c807db814fb00c997ded4878c4 apart from lines 172 and 175 which likely slipped through the simultaneous changes.

[tests] Make random seed logged and settable

This allows tests which use randomness to be reproducibly run on failure.

a407b6fdf3

jnewbery force-pushed on May 9, 2019

jonatack commented at 2:46 PM on May 11, 2019: member

re-ACK a407b6fdf34f77eb347378674da9cf80394897de

Terminal output:

test/functional/feature_help.py -l debug
2019-05-11T13:54:35.602000Z TestFramework (DEBUG): PRNG seed is: 623709155939669242

test/functional/feature_help.py --loglevel debug --randomseed 3139909
2019-05-11T13:54:42.522000Z TestFramework (DEBUG): User supplied random seed 3139909
2019-05-11T13:54:42.523000Z TestFramework (DEBUG): PRNG seed is: 3139909

Log file:

test/functional/feature_help.py 
2019-05-11T14:41:33.674000Z TestFramework (DEBUG): PRNG seed is: 2681408364687339094

test/functional/feature_help.py --randomseed=313990913
2019-05-11T14:39:22.451000Z TestFramework (DEBUG): User supplied random seed 313990913
2019-05-11T14:39:22.451000Z TestFramework (DEBUG): PRNG seed is: 313990913

Log file with test runner:

test/functional/test_runner.py feature_help
2019-05-11T14:43:46.690000Z TestFramework (DEBUG): PRNG seed is: 7494161081645491101

test/functional/test_runner.py feature_help --randomseed=195
2019-05-11T14:45:09.409000Z TestFramework (DEBUG): User supplied random seed 195
2019-05-11T14:45:09.409000Z TestFramework (DEBUG): PRNG seed is: 195

jb55 commented at 7:06 AM on May 14, 2019: member

great! utACK a407b6fdf34f77eb347378674da9cf80394897de

MarcoFalke merged this on May 14, 2019

MarcoFalke closed this on May 14, 2019

MarcoFalke referenced this in commit 3503a69ba2 on May 14, 2019

jnewbery deleted the branch on May 14, 2019

deadalnix referenced this in commit 524cc44787 on Jul 30, 2020

PastaPastaPasta referenced this in commit 89083db11c on Sep 11, 2021

PastaPastaPasta referenced this in commit b8f7cbca0b on Sep 11, 2021

PastaPastaPasta referenced this in commit 494c4ad1f0 on Sep 12, 2021

DrahtBot locked this on Dec 16, 2021