Test for ambiguous system ordering in CI (#13950)

Progress towards https://github.com/bevyengine/bevy/issues/7386.

Following discussion
https://discord.com/channels/691052431525675048/1253260494538539048/1253387942311886960

This Pull Request adds an example to detect system order ambiguities,
and also asserts none exist.

A lot of schedules are ignored in ordered to have the test passing, we
should thrive to make them pass, but in other pull requests.

<details><summary>example output <b>summary</b>, without ignored
schedules</summary>
<p>

```txt
$ cargo run --example ambiguity_detection 2>&1 | grep -C 1 "pairs of syst"
2024-06-21T13:17:55.776585Z  WARN bevy_ecs::schedule::schedule: Schedule First has ambiguities.
1 pairs of systems with conflicting data access have indeterminate execution order. Consider adding `before`, `after`, or `ambiguous_with` relationships between these:
 -- bevy_time::time_system (in set TimeSystem) and bevy_ecs::event::event_update_system (in set EventUpdates)
--
2024-06-21T13:17:55.782265Z  WARN bevy_ecs::schedule::schedule: Schedule PreUpdate has ambiguities.
11 pairs of systems with conflicting data access have indeterminate execution order. Consider adding `before`, `after`, or `ambiguous_with` relationships between these:
 -- bevy_pbr::prepass::update_mesh_previous_global_transforms and bevy_asset::server::handle_internal_asset_events
--
2024-06-21T13:17:55.809516Z  WARN bevy_ecs::schedule::schedule: Schedule PostUpdate has ambiguities.
63 pairs of systems with conflicting data access have indeterminate execution order. Consider adding `before`, `after`, or `ambiguous_with` relationships between these:
 -- bevy_ui::accessibility::image_changed and bevy_ecs::schedule::executor::apply_deferred
--
2024-06-21T13:17:55.816287Z  WARN bevy_ecs::schedule::schedule: Schedule Last has ambiguities.
3 pairs of systems with conflicting data access have indeterminate execution order. Consider adding `before`, `after`, or `ambiguous_with` relationships between these:
 -- bevy_gizmos::update_gizmo_meshes<bevy_gizmos::aabb::AabbGizmoConfigGroup> (in set UpdateGizmoMeshes) and bevy_gizmos::update_gizmo_meshes<bevy_gizmos::light::LightGizmoConfigGroup> (in set UpdateGizmoMeshes)
--
2024-06-21T13:17:55.831074Z  WARN bevy_ecs::schedule::schedule: Schedule ExtractSchedule has ambiguities.
296 pairs of systems with conflicting data access have indeterminate execution order. Consider adding `before`, `after`, or `ambiguous_with` relationships between these:
 -- bevy_render::extract_component::extract_components<bevy_sprite::SpriteSource> and bevy_render::render_asset::extract_render_asset<bevy_sprite::mesh2d::material::PreparedMaterial2d<bevy_sprite::mesh2d::color_material::ColorMaterial>>
```

</p>
</details> 

To try locally: 
```sh
CI_TESTING_CONFIG="./.github/example-run/ambiguity_detection.ron" cargo run --example ambiguity_detection --features "bevy_ci_testing,trace,trace_chrome"
```

---------

Co-authored-by: Jan Hohenheim <jan@hohenheim.ch>
This commit is contained in:
Thierry Berger 2024-07-17 23:05:48 +02:00 committed by GitHub
parent fa855f7974
commit 26fc4c7198
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 144 additions and 2 deletions

View file

@ -0,0 +1,2 @@
(
)

View file

@ -365,6 +365,7 @@ ron = "0.8.0"
flate2 = "1.0"
serde = { version = "1", features = ["derive"] }
bytemuck = "1.7"
bevy_render = { path = "crates/bevy_render", version = "0.15.0-dev", default-features = false }
# Needed to poll Task examples
futures-lite = "2.0.1"
async-std = "1.12"
@ -2980,6 +2981,14 @@ description = "Demonstrates customizing default window settings"
category = "Window"
wasm = true
[[example]]
name = "ambiguity_detection"
path = "tests/ecs/ambiguity_detection.rs"
doc-scrape-examples = true
[package.metadata.example.ambiguity_detection]
hidden = true
[[example]]
name = "resizing"
path = "tests/window/resizing.rs"

View file

@ -6,6 +6,7 @@ mod systems;
pub use self::config::*;
use bevy_app::prelude::*;
use bevy_ecs::schedule::IntoSystemConfigs;
use bevy_time::TimeUpdateStrategy;
use std::time::Duration;
@ -46,9 +47,11 @@ impl Plugin for CiTestingPlugin {
fixed_frame_time,
)));
}
app.add_event::<CiTestingCustomEvent>()
.insert_resource(config)
.add_systems(Update, systems::send_events);
.add_systems(
Update,
systems::send_events.before(bevy_window::close_when_requested),
);
}
}

View file

@ -0,0 +1,128 @@
//! A test to confirm that `bevy` doesn't regress its system ambiguities count when using [`DefaultPlugins`].
//! This is run in CI.
use bevy::{
ecs::schedule::{InternedScheduleLabel, LogLevel, ScheduleBuildSettings, ScheduleLabel},
prelude::*,
utils::HashMap,
};
use bevy_render::{pipelined_rendering::RenderExtractApp, Render, RenderApp};
/// FIXME: bevy should not have any ambiguities, but it takes time to clean these up,
/// so we're just ignoring those for now.
///
/// See [#7386](https://github.com/bevyengine/bevy/issues/7386) for relevant issue.
pub fn get_ignored_ambiguous_system_schedules() -> Vec<Box<dyn ScheduleLabel>> {
vec![
Box::new(First),
Box::new(PreUpdate),
Box::new(Update),
Box::new(PostUpdate),
Box::new(Last),
Box::new(ExtractSchedule),
Box::new(Render),
]
}
/// A test to confirm that `bevy` doesn't regress its system ambiguities count when using [`DefaultPlugins`].
/// This is run in CI.
pub fn main() {
let mut app = App::new();
app.add_plugins(DefaultPlugins);
let sub_app = app.main_mut();
configure_ambiguity_detection(sub_app);
let sub_app = app.sub_app_mut(RenderApp);
configure_ambiguity_detection(sub_app);
let sub_app = app.sub_app_mut(RenderExtractApp);
configure_ambiguity_detection(sub_app);
app.finish();
app.cleanup();
app.update();
let sub_app = app.main();
let ignored_schedules = get_ignored_ambiguous_system_schedules();
let ambiguities = count_ambiguities(sub_app);
let mut unexpected_ambiguities = vec![];
for (&label, &count) in ambiguities.0.iter() {
if ignored_schedules
.iter()
.any(|label_to_ignore| **label_to_ignore == *label)
{
continue;
}
if count != 0 {
unexpected_ambiguities.push(label);
}
}
assert_eq!(
unexpected_ambiguities.len(),
0,
"Main app has unexpected ambiguities among these schedules: {:?}.\n\
More Details:\n{:#?}",
unexpected_ambiguities,
ambiguities
);
let total_ambiguities = ambiguities.total();
assert_eq!(
total_ambiguities, 82,
"Main app does not have an expected conflicting systems count, \
you might consider verifying if it's normal, or change the expected number.\n\
Details:\n{:#?}",
ambiguities
);
// RenderApp is not checked here, because it is not within the App at this point.
let sub_app = app.sub_app(RenderExtractApp);
let ambiguities = count_ambiguities(sub_app);
let total_ambiguities = ambiguities.total();
assert_eq!(
total_ambiguities, 0,
"RenderExtractApp contains conflicting systems.",
);
}
/// Contains the number of conflicting systems per schedule.
#[derive(Debug, Deref, DerefMut)]
struct AmbiguitiesCount(pub HashMap<InternedScheduleLabel, usize>);
impl AmbiguitiesCount {
fn total(&self) -> usize {
self.values().sum()
}
}
fn configure_ambiguity_detection(sub_app: &mut SubApp) {
let ignored_ambiguous_systems = get_ignored_ambiguous_system_schedules();
let mut schedules = sub_app.world_mut().resource_mut::<Schedules>();
for (_, schedule) in schedules.iter_mut() {
if ignored_ambiguous_systems
.iter()
.any(|label| **label == *schedule.label())
{
// Note: you can remove this bypass to get full details about ambiguities.
continue;
}
schedule.set_build_settings(ScheduleBuildSettings {
ambiguity_detection: LogLevel::Warn,
use_shortnames: false,
..default()
});
}
}
/// Returns the number of conflicting systems per schedule.
fn count_ambiguities(sub_app: &SubApp) -> AmbiguitiesCount {
let schedules = sub_app.world().resource::<Schedules>();
let mut ambiguities = HashMap::new();
for (_, schedule) in schedules.iter() {
let ambiguities_in_schedule = schedule.graph().conflicting_systems().len();
ambiguities.insert(schedule.label(), ambiguities_in_schedule);
}
AmbiguitiesCount(ambiguities)
}