Conversation
src/generators/time.rs
Outdated
| /// ``` | ||
| pub fn instants() -> InstantGenerator { | ||
| InstantGenerator { | ||
| base: Instant::now(), |
There was a problem hiding this comment.
I think the base being now is a bad idea, because it means you get a different instant each time the test replays, so now your tests are possibly flaky. I'd rather have a fixed base (e.g. time zero, which is presumably the unix epoch?) and let the user set it with a base(Instant) method.
There was a problem hiding this comment.
Instant is intentionally opaque and Instant::now() is the only way to construct an Instant. If the goal is to generate arbitrary points in time then I think SystemTime is what we need.
e.g.
SystemTime::UNIX_EPOCH + std::time::Duration::from_nanoseconds(828273)
There was a problem hiding this comment.
Record of conversation we had on zulip: It does indeed seem to be impossible to get a fixed Instant in rust (which is for the record insane). I suggested that we can at least fix the instant once at test start time by having a global (mutex locked) base set to Instant::now. It gets us reproducibility within a single process, and usually gets us reproducibility across processes unless something bad is happening.
There was a problem hiding this comment.
this does in fact seem insane and I would like to think hard about the right thing to do here. @echoumcp1 I'd be happy to get Duration in in a separate PR while we decide on Instant
echoumcp1
changed the title
Duration and InstantDuration
3 participants
#118
Debating between the current implementation for
durations()or effectively making itintegers::<u64>().map(Duration::from_nanos)with maybe a wrapper for theDuration-typed bounds but no preference either way.