Merge pull request #436 from godot-rust/bugfix/test-failure

Smaller integration and unit-test updates (NaN eq, list failed, ...)

Author: Bromeon

.github/workflows/full-ci.yml

@@ -15,6 +15,7 @@ on:
 
 env:
   GDEXT_FEATURES: ''
+#  GDEXT_FEATURES: '--features godot/serde'
   RETRY: ${{ github.workspace }}/.github/other/retry.sh
 
   # LSan options: https://github.com/google/sanitizers/wiki/AddressSanitizerLeakSanitizer

godot-bindings/Cargo.toml

@@ -25,3 +25,7 @@ godot4-prebuilt = { optional = true, git = "https://github.com/godot-rust/godot4
 bindgen = { optional = true, version = "0.65", default-features = false, features = ["runtime"] }
 regex = { optional = true, version = "1.5.5", default-features = false, features = ["std", "unicode-gencat"] }
 which = { optional = true, version = "4" }
+
+[dev-dependencies]
+# For tests, we need regex unconditionally. Keep this in sync with above dependency.
+regex = { version = "1.5.5", default-features = false, features = ["std", "unicode-gencat"] }

godot-bindings/src/godot_version.rs

@@ -16,7 +16,7 @@ pub fn parse_godot_version(version_str: &str) -> Result<GodotVersion, Box<dyn Er
     // https://github.com/godot-rust/gdext/issues/118#issuecomment-1465748123
     // We assume that it's on a line of its own, but it may be surrounded by other lines.
     let regex = Regex::new(
-        r#"(?xm)
+        r"(?xm)
         # x: ignore whitespace and allow line comments (starting with `#`)
         # m: multi-line mode, ^ and $ match start and end of line
         ^
@@ -32,15 +32,17 @@ pub fn parse_godot_version(version_str: &str) -> Result<GodotVersion, Box<dyn Er
         (\.[^.]+)+?
         # Git commit SHA1, currently truncated to 9 chars, but accept the full thing
         (?:\.(?P<custom_rev>[a-f0-9]{9,40}))?
+        # Optional newline printed in some systems (e.g. Arch Linux, see #416)
+        (?:\\n)?
         $
-    "#,
+        ",
     )?;
 
     let fail = || format!("Version substring cannot be parsed: `{version_str}`");
     let caps = regex.captures(version_str).ok_or_else(fail)?;
 
     Ok(GodotVersion {
-        full_string: caps.get(0).unwrap().as_str().to_string(),
+        full_string: caps.get(0).unwrap().as_str().trim().to_string(),
         major: cap(&caps, "major")?.unwrap(),
         minor: cap(&caps, "minor")?.unwrap(),
         patch: cap(&caps, "patch")?.unwrap_or(0),
@@ -86,6 +88,7 @@ fn test_godot_versions() {
         ("4.0.beta8.mono.custom_build.b28ddd918", 4, 0, 0, "beta8", s("b28ddd918")),
         ("4.0.rc1.official.8843d9ad3", 4, 0, 0, "rc1", s("8843d9ad3")),
         ("4.0.stable.arch_linux", 4, 0, 0, "stable", None),
+        ("4.1.1.stable.arch_linux\n", 4, 1, 1, "stable", None),
         // Output from 4.0.stable on MacOS in debug mode:
         // https://github.com/godotengine/godot/issues/74906
         ("arguments
@@ -104,7 +107,7 @@ Current path: /Users/runner/work/gdext/gdext/godot-core
     for (full, major, minor, patch, status, custom_rev) in good_versions {
         let expected = GodotVersion {
             // Version line is last in every test at the moment.
-            full_string: full.lines().last().unwrap().to_owned(),
+            full_string: full.lines().last().unwrap().trim().to_owned(),
             major,
             minor,
             patch,

godot-bindings/src/lib.rs

@@ -40,6 +40,10 @@ pub struct GodotVersion {
 // ----------------------------------------------------------------------------------------------------------------------------------------------
 // Regenerate all files
 
+// This file is explicitly included in unit tests. Needs regex dependency.
+#[cfg(test)]
+mod godot_version;
+
 #[cfg(feature = "custom-godot")]
 #[path = ""]
 mod custom {

godot-core/src/builtin/callable.rs

@@ -425,9 +425,4 @@ mod custom_callable {
         w.name.clone().move_string_ptr(r_out);
         *r_is_valid = true as sys::GDExtensionBool;
     }
-
-    pub fn unqualified_type_name<T>() -> &'static str {
-        let type_name = std::any::type_name::<T>();
-        type_name.split("::").last().unwrap()
-    }
 }

godot-core/src/builtin/dictionary.rs

@@ -142,6 +142,7 @@ impl Dictionary {
     }
 
     /// Returns a 32-bit integer hash value representing the dictionary and its contents.
+    #[must_use]
     pub fn hash(&self) -> u32 {
         self.as_inner().hash().try_into().unwrap()
     }

godot-ffi/src/toolbox.rs

@@ -138,6 +138,13 @@ pub fn hash_value<T: std::hash::Hash>(t: &T) -> u64 {
     hasher.finish()
 }
 
+/*
+pub fn unqualified_type_name<T>() -> &'static str {
+    let type_name = std::any::type_name::<T>();
+    type_name.split("::").last().unwrap()
+}
+*/
+
 // ----------------------------------------------------------------------------------------------------------------------------------------------
 // Private helpers
 

itest/godot/ManualFfiTests.gd

@@ -9,11 +9,11 @@ func test_missing_init():
 
 	var expr = Expression.new()
 	var error = expr.parse("WithoutInit.new()")
-	if !assert_eq(error, OK, "Failed to parse dynamic expression"):
+	if not assert_eq(error, OK, "Failed to parse dynamic expression"):
 		return
 
 	var instance = expr.execute()
-	if !assert_that(!expr.has_execute_failed(), "Failed to evaluate dynamic expression"):
+	if not assert_that(!expr.has_execute_failed(), "Failed to evaluate dynamic expression"):
 		return
 
 	print("[GD] WithoutInit is: ", instance)

itest/godot/TestRunner.gd

@@ -92,7 +92,7 @@ class GDScriptExecutableTestCase extends GDScriptTestCase:
 		# This is a no-op if the suite doesn't have this property.
 		suite.set("_assertion_failed", false)
 		var result = suite.call(method_name)
-		var ok: bool = (result == true || result == null) && !suite.get("_assertion_failed")
+		var ok: bool = (result == true or result == null) and not suite.get("_assertion_failed")
 		return ok
 
 # Hardcoded test case used for special cases where the standard testing API is not sufficient.

itest/rust/src/builtin_tests/containers/dictionary_test.rs

@@ -95,6 +95,7 @@ fn dictionary_clone() {
         "foo": 0,
         "bar": subdictionary.clone()
     };
+
     #[allow(clippy::redundant_clone)]
     let clone = dictionary.clone();
     Dictionary::from_variant(&clone.get("bar").unwrap()).insert("final", 4);
@@ -103,12 +104,33 @@ fn dictionary_clone() {
 
 #[itest]
 fn dictionary_hash() {
-    let dictionary = dict! {
+    use godot::builtin::Vector2i;
+
+    let a = dict! {
         "foo": 0,
         "bar": true,
-        "baz": "foobar"
+        (Vector2i::new(4, -1)): "foobar",
+    };
+    let b = dict! {
+        "foo": 0,
+        "bar": true,
+        (Vector2i::new(4, -1)): "foobar" // No comma to test macro.
     };
-    dictionary.hash();
+    let c = dict! {
+        "foo": 0,
+        (Vector2i::new(4, -1)): "foobar",
+        "bar": true,
+    };
+
+    assert_eq!(a.hash(), b.hash(), "equal dictionaries have same hash");
+    assert_ne!(
+        a.hash(),
+        c.hash(),
+        "dictionaries with reordered content have different hash"
+    );
+
+    // NaNs are not equal (since Godot 4.2) but share same hash.
+    assert_eq!(dict! {772: f32::NAN}.hash(), dict! {772: f32::NAN}.hash());
 }
 
 #[itest]
@@ -328,8 +350,13 @@ fn dictionary_keys_values() {
 #[itest]
 fn dictionary_equal() {
     assert_eq!(dict! {"foo": "bar"}, dict! {"foo": "bar"});
-    assert_eq!(dict! {1: f32::NAN}, dict! {1: f32::NAN}); // yes apparently Godot considers these equal
     assert_ne!(dict! {"foo": "bar"}, dict! {"bar": "foo"});
+
+    // Changed in https://github.com/godotengine/godot/pull/74588.
+    #[cfg(before_api = "4.2")]
+    assert_eq!(dict! {1: f32::NAN}, dict! {1: f32::NAN});
+    #[cfg(since_api = "4.2")]
+    assert_ne!(dict! {1: f32::NAN}, dict! {1: f32::NAN});
 }
 
 #[itest]