Merge #164

164: Inject scene tree into `#[itest]` r=Bromeon a=Bromeon Also adds another test for `Gd::eq()` in the case of dead instances, and a stub for testing #23. Simplifies the proc-macro machinery further. Co-authored-by: Jan Haller <[email protected]>
godot-rust · Mar 9, 2023 · 2b72fd6 · 2b72fd6
2 parents ac5f78c + dcdb915
commit 2b72fd6
Show file tree

Hide file tree

Showing 9 changed files with 192 additions and 57 deletions.
diff --git a/godot-core/src/obj/gd.rs b/godot-core/src/obj/gd.rs
@@ -220,7 +220,7 @@ impl<T: GodotClass> Gd<T> {
         // Initialize instance ID cache
         let id = unsafe { interface_fn!(object_get_instance_id)(obj.obj_sys()) };
         let instance_id = InstanceId::try_from_u64(id)
-            .expect("instance ID must be non-zero at time of initialization");
+            .expect("Gd initialization failed; did you call share() on a dead instance?");
         obj.cached_instance_id.set(Some(instance_id));
 
         obj
@@ -452,7 +452,7 @@ where
         // If ref_counted returned None, that means the instance was destroyed
         assert!(
             ref_counted == Some(false) && self.is_instance_valid(),
-            "called free() on already destroyed obj"
+            "called free() on already destroyed object"
         );
 
         // This destroys the Storage instance, no need to run destructor again
@@ -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> {}
diff --git a/godot-macros/src/gdextension.rs b/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;

diff --git a/godot-macros/src/itest.rs b/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,
+    )
+}
diff --git a/godot-macros/src/util.rs b/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,
-        );
+        )
     }
 }
 
@@ -483,6 +496,13 @@ mod tests {
     }
 }
 
-pub(crate) fn path_is_single(path: &Vec<TokenTree>, expected: &str) -> bool {
+pub(crate) fn path_is_single(path: &[TokenTree], expected: &str) -> bool {
     path.len() == 1 && path[0].to_string() == expected
 }
+
+pub(crate) fn path_ends_with(path: &[TokenTree], expected: &str) -> bool {
+    // could also use TyExpr::as_path()
+    path.last()
+        .map(|last| last.to_string() == expected)
+        .unwrap_or(false)
+}
diff --git a/itest/godot/.godot/global_script_class_cache.cfg b/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"
-}]
+}])
diff --git a/itest/godot/TestRunner.gd b/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)

diff --git a/itest/rust/src/lib.rs b/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),
 }