Fix bevy_time tests occasionally failing on optimised Windows builds (#17349)
# Objective
Fix the `bevy_time` unit tests occasionally failing on optimised Windows
builds.
# Background
I noticed that the `bevy_time` unit tests would fail ~50% of the time
after enabling `opt-level=1` in config.toml, or adding `--release` to
cargo test.
```
> cargo test -p bevy_time --release
thread 'real::test::test_update' panicked at crates\bevy_time\src\real.rs:164:9:
assertion `left != right` failed
left: Some(Instant { t: 9458.0756664s })
right: Some(Instant { t: 9458.0756664s })
```
Disabling optimisations would fix the issue, as would switching from Windows to Linux.
The failing path is roughly:
```rust
let mut time = Time::<Real>::new(Instant::now());
time.update();
time.update();
assert_ne!(time.last_update(), time.first_update());
```
Which kinda boils down to:
```rust
let left = Instant::now();
let right = Instant::now();
assert_ne!(left, right);
```
So the failure seems legit, since there's no guarantee that `Instant::now()` increases between calls.
I suspect it only triggers with a combination of Windows + fast CPU + optimisations (Windows has a lower resolution clock than Linux/MacOS). That would explain why it doesn't fail on the Bevy Github CI (optimisations disabled, and I'm guessing the runner CPUs are clocked lower).
# Solution
Make sure `Instant::now()` has increased before calling `time.update()`.
I also considered:
1. Change the unit tests to accept `Instant:now()` not increasing.
- In retrospect this is maybe the better change?
- There's other unit tests that cover time increasing.
- Could also add a deterministic test for zero delta updates.
- I can switch the PR to this if desired.
2. Avoid any paths that hit `Instant::now()` in unit tests.
- Arguably unit tests should always be deterministic.
- But that would mean a bunch of paths aren't tested.
## Testing
`cargo test -p bevy_time --release`
## System Info
`os: "Windows 10 Pro", kernel: "19045", cpu: "AMD Ryzen 9 7900 12-Core Processor", core_count: "12", memory: "63.2 GiB"`
Also tested on same computer with Linux pop-os 6.9.3.
Co-authored-by: François Mockers <mockersf@gmail.com>
This commit is contained in:
parent
141b7673ab
commit
83b2265673
@ -139,6 +139,18 @@ impl Time<Real> {
|
||||
mod test {
|
||||
use super::*;
|
||||
|
||||
// Waits until Instant::now() has increased.
|
||||
//
|
||||
// ```
|
||||
// let previous = Instant::now();
|
||||
// wait();
|
||||
// assert!(Instant::now() > previous);
|
||||
// ```
|
||||
fn wait() {
|
||||
let start = Instant::now();
|
||||
while Instant::now() <= start {}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_update() {
|
||||
let startup = Instant::now();
|
||||
@ -150,6 +162,7 @@ mod test {
|
||||
assert_eq!(time.delta(), Duration::ZERO);
|
||||
assert_eq!(time.elapsed(), Duration::ZERO);
|
||||
|
||||
wait();
|
||||
time.update();
|
||||
|
||||
assert_ne!(time.first_update(), None);
|
||||
@ -157,6 +170,7 @@ mod test {
|
||||
assert_eq!(time.delta(), Duration::ZERO);
|
||||
assert_eq!(time.elapsed(), Duration::ZERO);
|
||||
|
||||
wait();
|
||||
time.update();
|
||||
|
||||
assert_ne!(time.first_update(), None);
|
||||
@ -165,6 +179,7 @@ mod test {
|
||||
assert_ne!(time.delta(), Duration::ZERO);
|
||||
assert_eq!(time.elapsed(), time.delta());
|
||||
|
||||
wait();
|
||||
let prev_elapsed = time.elapsed();
|
||||
time.update();
|
||||
|
||||
@ -177,6 +192,7 @@ mod test {
|
||||
let startup = Instant::now();
|
||||
let mut time = Time::<Real>::new(startup);
|
||||
|
||||
wait();
|
||||
let first_update = Instant::now();
|
||||
time.update_with_instant(first_update);
|
||||
|
||||
@ -186,6 +202,7 @@ mod test {
|
||||
assert_eq!(time.delta(), Duration::ZERO);
|
||||
assert_eq!(time.elapsed(), Duration::ZERO);
|
||||
|
||||
wait();
|
||||
let second_update = Instant::now();
|
||||
time.update_with_instant(second_update);
|
||||
|
||||
@ -194,6 +211,7 @@ mod test {
|
||||
assert_eq!(time.delta(), second_update - first_update);
|
||||
assert_eq!(time.elapsed(), second_update - first_update);
|
||||
|
||||
wait();
|
||||
let third_update = Instant::now();
|
||||
time.update_with_instant(third_update);
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user