Merge pull request #1203 from godot-rust/qol/failed-gdscript-calls

Verify that marshalling errors cause failed *GDScript* function

Author: Bromeon

itest/godot/ManualFfiTests.gd

@@ -92,9 +92,8 @@ func test_export_dyn_gd_should_fail_for_wrong_type():
 	var dyn_gd_exporter = RefcDynGdVarDeclarer.new()
 	var refc = RefcHealth.new()
 
-	disable_error_messages()
-	dyn_gd_exporter.second = refc # Should fail.
-	enable_error_messages()
+	expect_fail()
+	dyn_gd_exporter.second = refc # Causes current function to fail. Following code unreachable.
 
 	assert_fail("`DynGdExporter.second` should only accept NodeHealth and only if it implements `InstanceIdProvider` trait")
 
@@ -310,19 +309,19 @@ func test_custom_property_wrong_values_1():
 		return
 
 	var has_property: HasCustomProperty = HasCustomProperty.new()
-	disable_error_messages()
-	has_property.some_c_style_enum = 10 # Should fail.
-	enable_error_messages()
+	expect_fail()
+	has_property.some_c_style_enum = 10 # Causes current function to fail. Following code unreachable.
+
 	assert_fail("HasCustomProperty.some_c_style_enum should only accept integers in the range `(0 ..= 2)`")
 
 func test_custom_property_wrong_values_2():
 	if runs_release():
 		return
 
 	var has_property: HasCustomProperty = HasCustomProperty.new()
-	disable_error_messages()
-	has_property.not_exportable = {"a": "hello", "b": Callable()} # Should fail.
-	enable_error_messages()
+	expect_fail()
+	has_property.not_exportable = {"a": "hello", "b": Callable()} # Causes current function to fail. Following code unreachable.
+
 	assert_fail("HasCustomProperty.not_exportable should only accept dictionaries with float values")
 
 func test_option_export():
@@ -482,3 +481,58 @@ func test_renamed_func_get_set():
 	assert_eq(obj.f1(), 84)
 
 	obj.free()
+
+# -----------------------------------------------------------------------------------------------------------------------------------------------
+# Tests below verify the following:
+# Calling a typed Rust function with a Variant that cannot be converted to the Rust type will cause a failed function call on _GDScript_ side,
+# meaning the GDScript function aborts immediately. This happens because a `Variant -> T` conversion occurs dynamically *on GDScript side*,
+# before the Rust function is called.In contrast, panics inside the Rust function (e.g. variant.to::<T>()) just cause the *Rust* function to fail.
+#
+# Store arguments as Variant, as GDScript wouldn't parse script otherwise. Results in varcall being used.
+
+func test_marshalling_fail_variant_type():
+	if runs_release():
+		return
+
+	# Expects Object, pass GString.
+	var obj := ObjectTest.new()
+	var arg: Variant = "not an object"
+	expect_fail()
+	obj.pass_object(arg) # Causes current function to fail. Following code unreachable.
+
+	assert_fail("GDScript function should fail after marshalling error (bad variant type)")
+
+func test_marshalling_fail_non_null():
+	if runs_release():
+		return
+
+	# Expects Object, pass null.
+	var obj := ObjectTest.new()
+
+	expect_fail()
+	obj.pass_object(null) # Causes current function to fail. Following code unreachable.
+
+	assert_fail("GDScript function should fail after marshalling error (required non-null)")
+
+func test_marshalling_fail_integer_overflow():
+	if runs_release():
+		return
+
+	# Expects i32. This overflows.
+	var obj := ObjectTest.new()
+	var arg: Variant = 9223372036854775807
+
+	expect_fail()
+	obj.pass_i32(arg) # Causes current function to fail. Following code unreachable.
+
+	assert_fail("GDScript function should fail after marshalling error (int overflow)")
+
+func test_marshalling_continues_on_panic():
+	mark_test_pending()
+
+	# Expects i32. This overflows.
+	var obj := ObjectTest.new()
+	var result = obj.cause_panic() # Fails in Rust, current function continues.
+
+	assert_eq(result, Vector3.ZERO, "Default value returned on failed function call")
+	mark_test_succeeded()

itest/godot/TestSuite.gd

@@ -12,20 +12,22 @@ var _pending: bool = false
 # -----------------------------------------------------------------------------------------------------------------------------------------------
 # Public API, called by the test runner.
 
-func reset_state():
+func reset_state() -> void:
 	_assertion_failed = false
 	_pending = false
 
+	# Note: some tests disable error messages, but they are re-enabled by the Rust test runner before each test.
+
 func is_test_failed() -> bool:
 	return _assertion_failed
 
 # -----------------------------------------------------------------------------------------------------------------------------------------------
 # Protected API, called by individual test .gd files.
 
-func print_newline():
+func print_newline() -> void:
 	printerr()
 
-func print_error(s: String):
+func print_error(s: String) -> void:
 	push_error(s)
 
 ## Asserts that `what` is `true`, but does not abort the test. Returns `what` so you can return
@@ -62,7 +64,7 @@ func assert_eq(actual, expected, message: String = "") -> bool:
 ## Pre-emptively mark this test as "failed unless confirmed success". Use mark_test_succeeded() to rollback if test actually succeeds.
 ##
 ## For situations where statements coming after this will abort the test function without reliable ways to detect.
-func mark_test_pending():
+func mark_test_pending() -> void:
 	if _pending:
 		push_error("Test is already pending.")
 		_assertion_failed = true
@@ -73,7 +75,7 @@ func mark_test_pending():
 	#print("Test will fail unless rolled back: ", message)
 
 ## Roll back the failure assumption if the test actually succeeded.
-func mark_test_succeeded():
+func mark_test_succeeded() -> void:
 	if not _pending:
 		push_error("Cannot mark test as succeeded, test was not marked as pending.")
 		_assertion_failed = true
@@ -82,17 +84,15 @@ func mark_test_succeeded():
 	_pending = false
 	_assertion_failed = false
 
-# Disable error message printing from godot. 
-#
-# Error messages are always re-enabled by the rust test runner after a test has been run.
-func disable_error_messages():
-	Engine.print_error_messages = false
+## Expects that one of the next statements will cause the calling GDScript function to abort.
+##
+## This should always be followed by an assert_fail() call at the end of the function.
+func expect_fail() -> void:
+	if runs_release():
+		push_warning("Release builds do not have proper error handling, the calling test is likely to not work as expected.")
 
-# Enable error message printing from godot. 
-# 
-# Error messages are always re-enabled by the rust test runner after a test has been run.
-func enable_error_messages():
-	Engine.print_error_messages = true
+	# Note: error messages are re-enabled by the Rust test runner before each test.
+	Engine.print_error_messages = false
 
 # Asserts that the test failed to reach this point. You should disable error messages before running code 
 # that is expected to print an error message that would otherwise cause the CI to report failure.

itest/rust/src/object_tests/object_test.rs

@@ -1023,6 +1023,14 @@ pub mod object_test_gd {
             // Forward compat: .upcast() here becomes a breaking change if we generalize AsArg to include derived->base conversions.
             array![&Self::return_self().upcast()]
         }
+
+        #[func]
+        fn pass_i32(&self, _i: i32) {}
+
+        #[func]
+        fn cause_panic(&self) -> Vector3 {
+            panic!("Rust panics")
+        }
     }
 
     // ----------------------------------------------------------------------------------------------------------------------------------------------