Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

port old ambiguity tests over #9617

Merged
merged 1 commit into from
Aug 29, 2023

Conversation

hymm
Copy link
Contributor

@hymm hymm commented Aug 29, 2023

Objective

  • Some of the old ambiguity tests didn't get ported over during schedule v3.

Solution

  • Port over tests from
    #[cfg(test)]
    mod tests {
    // Required to make the derive macro behave
    use crate as bevy_ecs;
    use crate::event::Events;
    use crate::prelude::*;
    #[derive(Resource)]
    struct R;
    #[derive(Component)]
    struct A;
    #[derive(Component)]
    struct B;
    // An event type
    struct E;
    fn empty_system() {}
    fn res_system(_res: Res<R>) {}
    fn resmut_system(_res: ResMut<R>) {}
    fn nonsend_system(_ns: NonSend<R>) {}
    fn nonsendmut_system(_ns: NonSendMut<R>) {}
    fn read_component_system(_query: Query<&A>) {}
    fn write_component_system(_query: Query<&mut A>) {}
    fn with_filtered_component_system(_query: Query<&mut A, With<B>>) {}
    fn without_filtered_component_system(_query: Query<&mut A, Without<B>>) {}
    fn event_reader_system(_reader: EventReader<E>) {}
    fn event_writer_system(_writer: EventWriter<E>) {}
    fn event_resource_system(_events: ResMut<Events<E>>) {}
    fn read_world_system(_world: &World) {}
    fn write_world_system(_world: &mut World) {}
    // Tests for conflict detection
    #[test]
    fn one_of_everything() {
    let mut world = World::new();
    world.insert_resource(R);
    world.spawn(A);
    world.init_resource::<Events<E>>();
    let mut test_stage = SystemStage::parallel();
    test_stage
    // nonsendmut system deliberately conflicts with resmut system
    .add_system(resmut_system)
    .add_system(write_component_system)
    .add_system(event_writer_system);
    test_stage.run(&mut world);
    assert_eq!(test_stage.ambiguity_count(&world), 0);
    }
    #[test]
    fn read_only() {
    let mut world = World::new();
    world.insert_resource(R);
    world.insert_non_send_resource(R);
    world.spawn(A);
    world.init_resource::<Events<E>>();
    let mut test_stage = SystemStage::parallel();
    test_stage
    .add_system(empty_system)
    .add_system(empty_system)
    .add_system(res_system)
    .add_system(res_system)
    .add_system(nonsend_system)
    .add_system(nonsend_system)
    .add_system(read_component_system)
    .add_system(read_component_system)
    .add_system(event_reader_system)
    .add_system(event_reader_system)
    .add_system(read_world_system)
    .add_system(read_world_system);
    test_stage.run(&mut world);
    assert_eq!(test_stage.ambiguity_count(&world), 0);
    }
    #[test]
    fn read_world() {
    let mut world = World::new();
    world.insert_resource(R);
    world.spawn(A);
    world.init_resource::<Events<E>>();
    let mut test_stage = SystemStage::parallel();
    test_stage
    .add_system(resmut_system)
    .add_system(write_component_system)
    .add_system(event_writer_system)
    .add_system(read_world_system);
    test_stage.run(&mut world);
    assert_eq!(test_stage.ambiguity_count(&world), 3);
    }
    #[test]
    fn resources() {
    let mut world = World::new();
    world.insert_resource(R);
    let mut test_stage = SystemStage::parallel();
    test_stage.add_system(resmut_system).add_system(res_system);
    test_stage.run(&mut world);
    assert_eq!(test_stage.ambiguity_count(&world), 1);
    }
    #[test]
    fn nonsend() {
    let mut world = World::new();
    world.insert_resource(R);
    world.insert_non_send_resource(R);
    let mut test_stage = SystemStage::parallel();
    test_stage
    .add_system(nonsendmut_system)
    .add_system(nonsend_system);
    test_stage.run(&mut world);
    assert_eq!(test_stage.ambiguity_count(&world), 1);
    }
    #[test]
    fn components() {
    let mut world = World::new();
    world.spawn(A);
    let mut test_stage = SystemStage::parallel();
    test_stage
    .add_system(read_component_system)
    .add_system(write_component_system);
    test_stage.run(&mut world);
    assert_eq!(test_stage.ambiguity_count(&world), 1);
    }
    #[test]
    #[ignore = "Known failing but fix is non-trivial: https://github.com/bevyengine/bevy/issues/4381"]
    fn filtered_components() {
    let mut world = World::new();
    world.spawn(A);
    let mut test_stage = SystemStage::parallel();
    test_stage
    .add_system(with_filtered_component_system)
    .add_system(without_filtered_component_system);
    test_stage.run(&mut world);
    assert_eq!(test_stage.ambiguity_count(&world), 0);
    }
    #[test]
    fn events() {
    let mut world = World::new();
    world.init_resource::<Events<E>>();
    let mut test_stage = SystemStage::parallel();
    test_stage
    // All of these systems clash
    .add_system(event_reader_system)
    .add_system(event_writer_system)
    .add_system(event_resource_system);
    test_stage.run(&mut world);
    assert_eq!(test_stage.ambiguity_count(&world), 3);
    }
    #[test]
    fn exclusive() {
    let mut world = World::new();
    world.insert_resource(R);
    world.spawn(A);
    world.init_resource::<Events<E>>();
    let mut test_stage = SystemStage::parallel();
    test_stage
    // All 3 of these conflict with each other
    .add_system(write_world_system)
    .add_system(write_world_system.at_end())
    .add_system(res_system.at_start())
    // These do not, as they're in different segments of the stage
    .add_system(write_world_system.at_start())
    .add_system(write_world_system.before_commands());
    test_stage.run(&mut world);
    assert_eq!(test_stage.ambiguity_count(&world), 3);
    }
    // Tests for silencing and resolving ambiguities
    #[test]
    fn before_and_after() {
    let mut world = World::new();
    world.init_resource::<Events<E>>();
    let mut test_stage = SystemStage::parallel();
    test_stage
    .add_system(event_reader_system.before(event_writer_system))
    .add_system(event_writer_system)
    .add_system(event_resource_system.after(event_writer_system));
    test_stage.run(&mut world);
    assert_eq!(test_stage.ambiguity_count(&world), 0);
    }
    #[test]
    fn ignore_all_ambiguities() {
    let mut world = World::new();
    world.insert_resource(R);
    world.insert_non_send_resource(R);
    let mut test_stage = SystemStage::parallel();
    test_stage
    .add_system(resmut_system.ignore_all_ambiguities())
    .add_system(res_system)
    .add_system(nonsend_system);
    test_stage.run(&mut world);
    assert_eq!(test_stage.ambiguity_count(&world), 0);
    }
    #[test]
    fn ambiguous_with_label() {
    let mut world = World::new();
    world.insert_resource(R);
    world.insert_non_send_resource(R);
    #[derive(SystemLabel)]
    struct IgnoreMe;
    let mut test_stage = SystemStage::parallel();
    test_stage
    .add_system(resmut_system.ambiguous_with(IgnoreMe))
    .add_system(res_system.label(IgnoreMe))
    .add_system(nonsend_system.label(IgnoreMe));
    test_stage.run(&mut world);
    assert_eq!(test_stage.ambiguity_count(&world), 0);
    }
    #[test]
    fn ambiguous_with_system() {
    let mut world = World::new();
    let mut test_stage = SystemStage::parallel();
    test_stage
    .add_system(write_component_system.ambiguous_with(read_component_system))
    .add_system(read_component_system);
    test_stage.run(&mut world);
    assert_eq!(test_stage.ambiguity_count(&world), 0);
    }
    fn system_a(_res: ResMut<R>) {}
    fn system_b(_res: ResMut<R>) {}
    fn system_c(_res: ResMut<R>) {}
    fn system_d(_res: ResMut<R>) {}
    fn system_e(_res: ResMut<R>) {}
    // Tests that the correct ambiguities were reported in the correct order.
    #[test]
    fn correct_ambiguities() {
    use super::*;
    let mut world = World::new();
    world.insert_resource(R);
    let mut test_stage = SystemStage::parallel();
    test_stage
    .add_system(system_a)
    .add_system(system_b)
    .add_system(system_c.ignore_all_ambiguities())
    .add_system(system_d.ambiguous_with(system_b))
    .add_system(system_e.after(system_a));
    test_stage.run(&mut world);
    let ambiguities = test_stage.ambiguities(&world);
    assert_eq!(
    ambiguities,
    vec![
    SystemOrderAmbiguity {
    system_names: [
    "bevy_ecs::schedule::ambiguity_detection::tests::system_a".to_string(),
    "bevy_ecs::schedule::ambiguity_detection::tests::system_b".to_string()
    ],
    conflicts: vec!["bevy_ecs::schedule::ambiguity_detection::tests::R".to_string()],
    segment: SystemStageSegment::Parallel,
    },
    SystemOrderAmbiguity {
    system_names: [
    "bevy_ecs::schedule::ambiguity_detection::tests::system_a".to_string(),
    "bevy_ecs::schedule::ambiguity_detection::tests::system_d".to_string()
    ],
    conflicts: vec!["bevy_ecs::schedule::ambiguity_detection::tests::R".to_string()],
    segment: SystemStageSegment::Parallel,
    },
    SystemOrderAmbiguity {
    system_names: [
    "bevy_ecs::schedule::ambiguity_detection::tests::system_b".to_string(),
    "bevy_ecs::schedule::ambiguity_detection::tests::system_e".to_string()
    ],
    conflicts: vec!["bevy_ecs::schedule::ambiguity_detection::tests::R".to_string()],
    segment: SystemStageSegment::Parallel,
    },
    SystemOrderAmbiguity {
    system_names: [
    "bevy_ecs::schedule::ambiguity_detection::tests::system_d".to_string(),
    "bevy_ecs::schedule::ambiguity_detection::tests::system_e".to_string()
    ],
    conflicts: vec!["bevy_ecs::schedule::ambiguity_detection::tests::R".to_string()],
    segment: SystemStageSegment::Parallel,
    },
    ]
    );
    }
    }
    with minimal changes
  • Make a method to convert the ambiguity conflicts to a string for easier verification of correct results.

@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events C-Code-Quality A section of code that is hard to understand or change labels Aug 29, 2023
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice to see these revived: thanks!

Copy link
Member

@JoJoJet JoJoJet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't know these got deleted, thanks! They were added in 0.9 (#6053), so looks like they only survived for one release before being yoten haha.

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Aug 29, 2023
@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Aug 29, 2023
Merged via the queue into bevyengine:main with commit da9a070 Aug 29, 2023
24 checks passed
@hymm hymm deleted the port-old-ambiguity-tests branch August 30, 2023 02:04
rdrpenguin04 pushed a commit to rdrpenguin04/bevy that referenced this pull request Jan 9, 2024
# Objective

- Some of the old ambiguity tests didn't get ported over during schedule
v3.

## Solution

- Port over tests from
https://github.com/bevyengine/bevy/blob/15ee98db8d1c6705111e0f11a8fc240ceaf9f2db/crates/bevy_ecs/src/schedule/ambiguity_detection.rs#L279-L612
with minimal changes
- Make a method to convert the ambiguity conflicts to a string for
easier verification of correct results.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Code-Quality A section of code that is hard to understand or change S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants