From 98954311b37d72265b07218182d5754ac99041a9 Mon Sep 17 00:00:00 2001 From: JoJoJet <21144246+JoJoJet@users.noreply.github.com> Date: Thu, 30 Mar 2023 17:47:36 -0400 Subject: [PATCH] Use actual tests for `SystemParam` regression tests (#8270) # Objective Our regression tests for `SystemParam` currently consist of a bunch of loosely dispersed struct definitions. This is messy, and doesn't fully test their functionality. ## Solution Group the struct definitions into functions annotated with `#[test]`. This not only makes the module more organized, but it allows us to call `assert_is_system`, which has the potential to catch some bugs that would have been missed with the old approach. Also, this approach is consistent with how `WorldQuery` regression tests are organized. --- crates/bevy_ecs/src/system/system_param.rs | 206 +++++++++++++-------- 1 file changed, 132 insertions(+), 74 deletions(-) diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index e3b7546150..94b01e4900 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -1563,116 +1563,174 @@ mod tests { use crate::{ self as bevy_ecs, // Necessary for the `SystemParam` Derive when used inside `bevy_ecs`. query::{ReadOnlyWorldQuery, WorldQuery}, - system::Query, + system::{assert_is_system, Query}, }; use std::marker::PhantomData; // Compile test for https://github.com/bevyengine/bevy/pull/2838. - #[derive(SystemParam)] - pub struct SpecialQuery< - 'w, - 's, - Q: WorldQuery + Send + Sync + 'static, - F: ReadOnlyWorldQuery + Send + Sync + 'static = (), - > { - _query: Query<'w, 's, Q, F>, + #[test] + fn system_param_generic_bounds() { + #[derive(SystemParam)] + pub struct SpecialQuery< + 'w, + 's, + Q: WorldQuery + Send + Sync + 'static, + F: ReadOnlyWorldQuery + Send + Sync + 'static = (), + > { + _query: Query<'w, 's, Q, F>, + } + + fn my_system(_: SpecialQuery<(), ()>) {} + assert_is_system(my_system); } // Compile tests for https://github.com/bevyengine/bevy/pull/6694. + #[test] + fn system_param_flexibility() { + #[derive(SystemParam)] + pub struct SpecialRes<'w, T: Resource> { + _res: Res<'w, T>, + } - #[derive(SystemParam)] - pub struct SpecialRes<'w, T: Resource> { - _res: Res<'w, T>, - } + #[derive(SystemParam)] + pub struct SpecialLocal<'s, T: FromWorld + Send + 'static> { + _local: Local<'s, T>, + } - #[derive(SystemParam)] - pub struct SpecialLocal<'s, T: FromWorld + Send + 'static> { - _local: Local<'s, T>, + #[derive(Resource)] + struct R; + + fn my_system(_: SpecialRes, _: SpecialLocal) {} + assert_is_system(my_system); } #[derive(Resource)] pub struct R; // Compile test for https://github.com/bevyengine/bevy/pull/7001. - #[derive(SystemParam)] - pub struct ConstGenericParam<'w, const I: usize>(Res<'w, R>); + #[test] + fn system_param_const_generics() { + #[derive(SystemParam)] + pub struct ConstGenericParam<'w, const I: usize>(Res<'w, R>); - // Compile test for https://github.com/bevyengine/bevy/pull/6867. - #[derive(SystemParam)] - pub struct LongParam<'w> { - _r0: Res<'w, R<0>>, - _r1: Res<'w, R<1>>, - _r2: Res<'w, R<2>>, - _r3: Res<'w, R<3>>, - _r4: Res<'w, R<4>>, - _r5: Res<'w, R<5>>, - _r6: Res<'w, R<6>>, - _r7: Res<'w, R<7>>, - _r8: Res<'w, R<8>>, - _r9: Res<'w, R<9>>, - _r10: Res<'w, R<10>>, - _r11: Res<'w, R<11>>, - _r12: Res<'w, R<12>>, - _r13: Res<'w, R<13>>, - _r14: Res<'w, R<14>>, - _r15: Res<'w, R<15>>, - _r16: Res<'w, R<16>>, + fn my_system(_: ConstGenericParam<0>, _: ConstGenericParam<1000>) {} + assert_is_system(my_system); } - #[allow(dead_code)] - fn long_system(_param: LongParam) { - crate::system::assert_is_system(long_system); + // Compile test for https://github.com/bevyengine/bevy/pull/6867. + #[test] + fn system_param_field_limit() { + #[derive(SystemParam)] + pub struct LongParam<'w> { + // Each field should be a distinct type so there will + // be an error if the derive messes up the field order. + _r0: Res<'w, R<0>>, + _r1: Res<'w, R<1>>, + _r2: Res<'w, R<2>>, + _r3: Res<'w, R<3>>, + _r4: Res<'w, R<4>>, + _r5: Res<'w, R<5>>, + _r6: Res<'w, R<6>>, + _r7: Res<'w, R<7>>, + _r8: Res<'w, R<8>>, + _r9: Res<'w, R<9>>, + _r10: Res<'w, R<10>>, + _r11: Res<'w, R<11>>, + _r12: Res<'w, R<12>>, + _r13: Res<'w, R<13>>, + _r14: Res<'w, R<14>>, + _r15: Res<'w, R<15>>, + _r16: Res<'w, R<16>>, + } + + fn long_system(_: LongParam) {} + assert_is_system(long_system); } // Compile test for https://github.com/bevyengine/bevy/pull/6919. // Regression test for https://github.com/bevyengine/bevy/issues/7447. - #[derive(SystemParam)] - struct IgnoredParam<'w, T: Resource, Marker: 'static> { - _foo: Res<'w, T>, - #[system_param(ignore)] - marker: PhantomData<&'w Marker>, - marker2: PhantomData<&'w Marker>, + #[test] + fn system_param_phantom_data() { + #[derive(SystemParam)] + struct IgnoredParam<'w, T: Resource, Marker: 'static> { + _foo: Res<'w, T>, + #[system_param(ignore)] + marker: PhantomData<&'w Marker>, + marker2: PhantomData<&'w Marker>, + } + + fn my_system(_: IgnoredParam, ()>) {} + assert_is_system(my_system); } // Compile tests for https://github.com/bevyengine/bevy/pull/6957. + #[test] + fn system_param_struct_variants() { + #[derive(SystemParam)] + pub struct UnitParam; - #[derive(SystemParam)] - pub struct UnitParam; + #[derive(SystemParam)] + pub struct TupleParam<'w, 's, R: Resource, L: FromWorld + Send + 'static>( + Res<'w, R>, + Local<'s, L>, + ); - #[derive(SystemParam)] - pub struct TupleParam<'w, 's, R: Resource, L: FromWorld + Send + 'static>( - Res<'w, R>, - Local<'s, L>, - ); - - #[derive(Resource)] - struct PrivateResource; + fn my_system(_: UnitParam, _: TupleParam, u32>) {} + assert_is_system(my_system); + } // Regression test for https://github.com/bevyengine/bevy/issues/4200. - #[derive(SystemParam)] - pub struct EncapsulatedParam<'w>(Res<'w, PrivateResource>); + #[test] + fn system_param_private_fields() { + #[derive(Resource)] + struct PrivateResource; + + #[derive(SystemParam)] + pub struct EncapsulatedParam<'w>(Res<'w, PrivateResource>); + + fn my_system(_: EncapsulatedParam) {} + assert_is_system(my_system); + } // Regression test for https://github.com/bevyengine/bevy/issues/7103. - #[derive(SystemParam)] - pub struct WhereParam<'w, 's, Q> - where - Q: 'static + WorldQuery, - { - _q: Query<'w, 's, Q, ()>, + #[test] + fn system_param_where_clause() { + #[derive(SystemParam)] + pub struct WhereParam<'w, 's, Q> + where + Q: 'static + WorldQuery, + { + _q: Query<'w, 's, Q, ()>, + } + + fn my_system(_: WhereParam<()>) {} + assert_is_system(my_system); } // Regression test for https://github.com/bevyengine/bevy/issues/1727. - #[derive(SystemParam)] - pub struct Collide<'w> { - _x: Res<'w, FetchState>, - } + #[test] + fn system_param_name_collision() { + #[derive(Resource)] + pub struct FetchState; - #[derive(Resource)] - pub struct FetchState; + #[derive(SystemParam)] + pub struct Collide<'w> { + _x: Res<'w, FetchState>, + } + + fn my_system(_: Collide) {} + assert_is_system(my_system); + } // Regression test for https://github.com/bevyengine/bevy/issues/8192. - #[derive(SystemParam)] - pub struct InvariantParam<'w, 's> { - _set: ParamSet<'w, 's, (Query<'w, 's, ()>,)>, + #[test] + fn system_param_invariant_lifetime() { + #[derive(SystemParam)] + pub struct InvariantParam<'w, 's> { + _set: ParamSet<'w, 's, (Query<'w, 's, ()>,)>, + } + + fn my_system(_: InvariantParam) {} + assert_is_system(my_system); } }