Inject Godot scene-tree into #[itest] cases

Also a few proc-macro refactorings

Author: Bromeon

godot-core/src/obj/gd.rs

@@ -671,3 +671,8 @@ impl<T: GodotClass> VariantMetadata for Gd<T> {
         ClassName::of::<T>()
     }
 }
+
+// Gd unwinding across panics does not invalidate any invariants;
+// its mutability is anyway present, in the Godot engine.
+impl<T: GodotClass> std::panic::UnwindSafe for Gd<T> {}
+impl<T: GodotClass> std::panic::RefUnwindSafe for Gd<T> {}

godot-macros/src/gdextension.rs

@@ -4,12 +4,14 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/.
  */
 
-use crate::util::{bail, ident, parse_kv_group, path_is_single, validate_impl, KvValue};
 use proc_macro2::TokenStream;
 use quote::quote;
 use venial::Declaration;
 
-pub fn transform(decl: Declaration) -> Result<TokenStream, venial::Error> {
+use crate::util::{bail, ident, validate_impl, KvParser};
+use crate::ParseResult;
+
+pub fn transform(decl: Declaration) -> ParseResult<TokenStream> {
     let mut impl_decl = match decl {
         Declaration::Impl(item) => item,
         _ => return bail("#[gdextension] can only be applied to trait impls", &decl),
@@ -23,18 +25,10 @@ pub fn transform(decl: Declaration) -> Result<TokenStream, venial::Error> {
         );
     }
 
-    let mut entry_point = None;
-    for attr in impl_decl.attributes.drain(..) {
-        if path_is_single(&attr.path, "gdextension") {
-            for (k, v) in parse_kv_group(&attr.value).expect("#[gdextension] has invalid arguments")
-            {
-                match (k.as_str(), v) {
-                    ("entry_point", KvValue::Ident(f)) => entry_point = Some(f),
-                    _ => return bail(format!("#[gdextension]: invalid argument `{k}`"), attr),
-                }
-            }
-        }
-    }
+    let drained_attributes = std::mem::take(&mut impl_decl.attributes);
+    let mut parser = KvParser::parse_required(&drained_attributes, "gdextension", &impl_decl)?;
+    let entry_point = parser.handle_ident("entry_point")?;
+    parser.finish()?;
 
     let entry_point = entry_point.unwrap_or_else(|| ident("gdextension_rust_init"));
     let impl_ty = &impl_decl.self_ty;

godot-macros/src/itest.rs

@@ -4,11 +4,12 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/.
  */
 
-use crate::util::{bail, KvParser};
-use crate::ParseResult;
 use proc_macro2::TokenStream;
-use quote::quote;
-use venial::Declaration;
+use quote::{quote, ToTokens};
+use venial::{Declaration, Error, FnParam, Function};
+
+use crate::util::{bail, path_ends_with, KvParser};
+use crate::ParseResult;
 
 pub fn transform(input_decl: Declaration) -> ParseResult<TokenStream> {
     let func = match input_decl {
@@ -18,14 +19,11 @@ pub fn transform(input_decl: Declaration) -> ParseResult<TokenStream> {
 
     // Note: allow attributes for things like #[rustfmt] or #[clippy]
     if func.generic_params.is_some()
-        || !func.params.is_empty()
+        || func.params.len() > 1
         || func.return_ty.is_some()
         || func.where_clause.is_some()
     {
-        return bail(
-            format!("#[itest] must be of form:  fn {}() {{ ... }}", func.name),
-            &func,
-        );
+        return bad_signature(&func);
     }
 
     let mut attr = KvParser::parse_required(&func.attributes, "itest", &func.name)?;
@@ -42,10 +40,27 @@ pub fn transform(input_decl: Declaration) -> ParseResult<TokenStream> {
 
     let test_name = &func.name;
     let test_name_str = func.name.to_string();
+
+    // Detect parameter name chosen by user, or unused fallback
+    let param = if let Some((param, _punct)) = func.params.first() {
+        if let FnParam::Typed(param) = param {
+            // Correct parameter type (crude macro check) -> reuse parameter name
+            if path_ends_with(&param.ty.tokens, "TestContext") {
+                param.to_token_stream()
+            } else {
+                return bad_signature(&func);
+            }
+        } else {
+            return bad_signature(&func);
+        }
+    } else {
+        quote! { __unused_context: &crate::TestContext }
+    };
+
     let body = &func.body;
 
     Ok(quote! {
-        pub fn #test_name() {
+        pub fn #test_name(#param) {
             #body
         }
 
@@ -59,3 +74,15 @@ pub fn transform(input_decl: Declaration) -> ParseResult<TokenStream> {
         });
     })
 }
+
+fn bad_signature(func: &Function) -> Result<TokenStream, Error> {
+    bail(
+        format!(
+            "#[itest] function must have one of these signatures:\
+                \n  fn {f}() {{ ... }}\
+                \n  fn {f}(ctx: &TestContext) {{ ... }}",
+            f = func.name
+        ),
+        &func,
+    )
+}

godot-macros/src/util.rs

@@ -67,24 +67,22 @@ pub(crate) struct KvParser {
     finished: bool,
 }
 
+#[allow(dead_code)] // some functions will be used later
 impl KvParser {
     /// Create a new parser which requires a `#[expected]` attribute.
     ///
     /// `context` is used for the span in error messages.
-    #[allow(dead_code)] // will be used later
     pub fn parse_required(
         attributes: &[Attribute],
         expected: &str,
         context: impl ToTokens,
     ) -> ParseResult<Self> {
         match Self::parse(attributes, expected) {
             Ok(Some(result)) => Ok(result),
-            Ok(None) => {
-                return bail(
-                    format!("expected attribute #[{expected}], but not present"),
-                    context,
-                )
-            }
+            Ok(None) => bail(
+                format!("expected attribute #[{expected}], but not present"),
+                context,
+            ),
             Err(e) => Err(e),
         }
     }
@@ -147,11 +145,26 @@ impl KvParser {
         }
     }
 
+    /// `#[attr(key="string", key2=123, key3=true)]`, with a given key being required
+    pub fn handle_ident_required(&mut self, key: &str) -> ParseResult<Ident> {
+        self.inner_required(key, "ident", Self::handle_ident)
+    }
+
     /// `#[attr(key="string", key2=123, key3=true)]`, with a given key being required
     pub fn handle_lit_required(&mut self, key: &str) -> ParseResult<String> {
-        match self.handle_lit(key) {
+        self.inner_required(key, "literal", Self::handle_lit)
+    }
+
+    fn inner_required<T, F>(&mut self, key: &str, context: &str, mut f: F) -> ParseResult<T>
+    where
+        F: FnMut(&mut Self, &str) -> ParseResult<Option<T>>,
+    {
+        match f(self, key) {
             Ok(Some(string)) => Ok(string),
-            Ok(None) => self.bail_key(key, "expected to have literal value, but is absent"),
+            Ok(None) => self.bail_key(
+                key,
+                &format!("expected to have {context} value, but is absent"),
+            ),
             Err(err) => Err(err),
         }
     }
@@ -171,21 +184,21 @@ impl KvParser {
             let keys = self.map.keys().cloned().collect::<Vec<_>>().join(", ");
 
             let s = if self.map.len() > 1 { "s" } else { "" }; // plural
-            return bail(
+            bail(
                 format!(
                     "#[{attr}]: unrecognized key{s}: {keys}",
                     attr = self.attr_name
                 ),
                 self.span,
-            );
+            )
         }
     }
 
     fn bail_key<R>(&self, key: &str, msg: &str) -> ParseResult<R> {
-        return bail(
+        bail(
             format!("#[{attr}]: key `{key}` {msg}", attr = self.attr_name),
             self.span,
-        );
+        )
     }
 }
 
@@ -486,3 +499,10 @@ mod tests {
 pub(crate) fn path_is_single(path: &Vec<TokenTree>, expected: &str) -> bool {
     path.len() == 1 && path[0].to_string() == expected
 }
+
+pub(crate) fn path_ends_with(path: &Vec<TokenTree>, expected: &str) -> bool {
+    // could also use .as_path()
+    path.last()
+        .map(|last| last.to_string() == expected)
+        .unwrap_or(false)
+}

itest/godot/.godot/global_script_class_cache.cfg

@@ -1,13 +1,13 @@
-list=[{
+list=Array[Dictionary]([{
 "base": &"RefCounted",
 "class": &"TestStats",
 "icon": "",
 "language": &"GDScript",
 "path": "res://TestStats.gd"
 }, {
-"base": &"Node",
+"base": &"RefCounted",
 "class": &"TestSuite",
 "icon": "",
 "language": &"GDScript",
 "path": "res://TestSuite.gd"
-}]
+}])

itest/godot/TestRunner.gd

@@ -33,7 +33,12 @@ func _ready():
 			if method_name.begins_with("test_"):
 				gdscript_tests.push_back(GDScriptTestCase.new(suite, method_name))
 
-	var success: bool = rust_runner.run_all_tests(gdscript_tests, gdscript_suites.size(), allow_focus)
+	var success: bool = rust_runner.run_all_tests(
+		gdscript_tests,
+		gdscript_suites.size(),
+		allow_focus,
+		self,
+	)
 
 	var exit_code: int = 0 if success else 1
 	get_tree().quit(exit_code)

itest/rust/src/lib.rs

@@ -4,7 +4,9 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/.
  */
 
+use godot::engine::Node;
 use godot::init::{gdextension, ExtensionLibrary};
+use godot::obj::Gd;
 use godot::sys;
 
 mod array_test;
@@ -89,6 +91,10 @@ fn collect_rust_tests() -> (Vec<RustTestCase>, usize, bool) {
     (tests, all_files.len(), is_focus_run)
 }
 
+pub struct TestContext {
+    scene_tree: Gd<Node>,
+}
+
 #[derive(Copy, Clone)]
 struct RustTestCase {
     name: &'static str,
@@ -98,5 +104,5 @@ struct RustTestCase {
     focused: bool,
     #[allow(dead_code)]
     line: u32,
-    function: fn(),
+    function: fn(&TestContext),
 }

itest/rust/src/object_test.rs

@@ -4,19 +4,21 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/.
  */
 
-use crate::{expect_panic, itest};
+use std::cell::RefCell;
+use std::mem;
+use std::rc::Rc;
+
 use godot::bind::{godot_api, GodotClass, GodotExt};
 use godot::builtin::{
     FromVariant, GodotString, StringName, ToVariant, Variant, VariantConversionError, Vector3,
 };
+use godot::engine::node::InternalMode;
 use godot::engine::{file_access, Area2D, Camera3D, FileAccess, Node, Node3D, Object, RefCounted};
 use godot::obj::{Base, Gd, InstanceId};
 use godot::obj::{Inherits, Share};
 use godot::sys::GodotFfi;
 
-use std::cell::RefCell;
-use std::mem;
-use std::rc::Rc;
+use crate::{expect_panic, itest, TestContext};
 
 // TODO:
 // * make sure that ptrcalls are used when possible (ie. when type info available; maybe GDScript integration test)
@@ -596,6 +598,17 @@ fn object_call_with_args() {
     node.free();
 }
 
+#[itest]
+fn object_get_scene_tree(ctx: &TestContext) {
+    let node = Node3D::new_alloc();
+
+    let mut tree = ctx.scene_tree.share();
+    tree.add_child(node.upcast(), false, InternalMode::INTERNAL_MODE_DISABLED);
+
+    let count = tree.get_child_count(false);
+    assert_eq!(count, 1);
+} // implicitly tested: node does not leak
+
 // ----------------------------------------------------------------------------------------------------------------------------------------------
 
 #[inline(never)] // force to move "out of scope", can trigger potential dangling pointer errors

itest/rust/src/runner.rs

@@ -4,11 +4,14 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/.
  */
 
+use std::time::{Duration, Instant};
+
 use godot::bind::{godot_api, GodotClass};
 use godot::builtin::{ToVariant, Variant, VariantArray};
+use godot::engine::Node;
+use godot::obj::Gd;
 
-use crate::RustTestCase;
-use std::time::{Duration, Instant};
+use crate::{RustTestCase, TestContext};
 
 #[derive(GodotClass, Debug)]
 #[class(init)]
@@ -28,6 +31,7 @@ impl IntegrationTests {
         gdscript_tests: VariantArray,
         gdscript_file_count: i64,
         allow_focus: bool,
+        scene_tree: Gd<Node>,
     ) -> bool {
         println!("{}Run{} Godot integration tests...", FMT_CYAN_BOLD, FMT_END);
 
@@ -50,7 +54,7 @@ impl IntegrationTests {
         }
 
         let clock = Instant::now();
-        self.run_rust_tests(rust_tests);
+        self.run_rust_tests(rust_tests, scene_tree);
         let rust_time = clock.elapsed();
         let gdscript_time = if !focus_run {
             self.run_gdscript_tests(gdscript_tests);
@@ -62,10 +66,12 @@ impl IntegrationTests {
         self.conclude(rust_time, gdscript_time, allow_focus)
     }
 
-    fn run_rust_tests(&mut self, tests: Vec<RustTestCase>) {
+    fn run_rust_tests(&mut self, tests: Vec<RustTestCase>, scene_tree: Gd<Node>) {
+        let ctx = TestContext { scene_tree };
+
         let mut last_file = None;
         for test in tests {
-            let outcome = run_rust_test(&test);
+            let outcome = run_rust_test(&test, &ctx);
 
             self.update_stats(&outcome);
             print_test(test.file.to_string(), test.name, outcome, &mut last_file);
@@ -160,14 +166,14 @@ const FMT_YELLOW: &str = "\x1b[33m";
 const FMT_RED: &str = "\x1b[31m";
 const FMT_END: &str = "\x1b[0m";
 
-fn run_rust_test(test: &RustTestCase) -> TestOutcome {
+fn run_rust_test(test: &RustTestCase, ctx: &TestContext) -> TestOutcome {
     if test.skipped {
         return TestOutcome::Skipped;
     }
 
     // Explicit type to prevent tests from returning a value
-    let success: Option<()> =
-        godot::private::handle_panic(|| format!("   !! Test {} failed", test.name), test.function);
+    let err_context = || format!("   !! Test {} failed", test.name);
+    let success: Option<()> = godot::private::handle_panic(err_context, || (test.function)(ctx));
 
     TestOutcome::from_bool(success.is_some())
 }