spmallette commented on code in PR #3303:
URL: https://github.com/apache/tinkerpop/pull/3303#discussion_r2787675072
##########
gremlin-go/driver/cucumber/cucumberSteps_test.go:
##########
@@ -588,174 +592,205 @@ func (tg *tinkerPopGraph)
theResultShouldBe(characterizedAs string, table *godog
}
}
+// compareMapEquals compares two maps for equality by checking if they have
the same length
+// and if every key-value pair in the actual map has a logically equal
counterpart in the expected map.
func compareMapEquals(expected map[interface{}]interface{}, actual
map[interface{}]interface{}) bool {
+ if len(actual) != len(expected) {
+ return false
+ }
for k, a := range actual {
var e interface{}
containsKey := false
for ke, ee := range expected {
- if fmt.Sprint(k) == fmt.Sprint(ke) {
+ if compareSingleEqual(k, ke) {
containsKey = true
e = ee
break
- } else {
- if reflect.ValueOf(k).Kind() == reflect.Ptr &&
- reflect.ValueOf(ke).Kind() ==
reflect.Ptr {
- switch k.(type) {
- case *gremlingo.Vertex:
- switch ke.(type) {
- case *gremlingo.Vertex:
- if
fmt.Sprint(*k.(*gremlingo.Vertex)) == fmt.Sprint(*ke.(*gremlingo.Vertex)) {
- containsKey =
true
- }
- default:
- // Not equal.
- }
- default:
- // If we are here we probably
need to implement an additional type like the Vertex above.
- if
fmt.Sprint(*k.(*interface{})) == fmt.Sprint(*ke.(*interface{})) {
- fmt.Println("WARNING:
Encountered unknown pointer type as map key.")
- containsKey = true
- }
- }
- if containsKey {
- e = ee
- break
- }
- }
}
}
if !containsKey {
fmt.Printf("Map comparison error: Failed to find key %s
in %v\n", k, expected)
return false
}
- if a == nil && e == nil {
- continue
- } else if a == nil || e == nil {
- // One value is nil, other is not. They are not equal.
- fmt.Printf("Map comparison error: One map has a nil
key, other does not.\n")
+ if !compareSingleEqual(a, e) {
+ fmt.Printf("Map comparison error: Expected != Actual
(%v!=%v)\n", e, a)
return false
- } else {
- switch reflect.TypeOf(a).Kind() {
- case reflect.Array, reflect.Slice:
- switch reflect.TypeOf(e).Kind() {
- case reflect.Array, reflect.Slice:
- // Compare arrays
- if
!compareListEqualsWithoutOrder(e.([]interface{}), a.([]interface{})) {
- return false
- }
- default:
- fmt.Printf("Map comparison error:
Expected type is Array/Slice, actual is %s.\n", reflect.TypeOf(a).Kind())
- return false
- }
- case reflect.Map:
- switch reflect.TypeOf(a).Kind() {
- case reflect.Map:
- // Compare maps
- if
!compareMapEquals(e.(map[interface{}]interface{}),
a.(map[interface{}]interface{})) {
- return false
- }
- default:
- fmt.Printf("Map comparison error:
Expected type is Map, actual is %s.\n", reflect.TypeOf(a).Kind())
- return false
- }
- default:
- if fmt.Sprint(a) != fmt.Sprint(e) {
- fmt.Printf("Map comparison error:
Expected != Actual (%s!=%s)\n", fmt.Sprint(a), fmt.Sprint(e))
- return false
- }
- }
}
}
return true
}
-func compareListEqualsWithoutOrder(expected []interface{}, actual
[]interface{}) bool {
- // This is a little weird, but there isn't a good solution to either of
these problems:
- // 1. Comparison of types in Go. No deep equals which
actually works properly. Needs to be done manually.
- // 2. In place deletion in a loop.
- // So to do in place deletion in a loop we can do the following:
- // 1. Loop from back to wrong (don't need to worry about
deleted indices that way.
- // 2. Create a new slice with the index removed when we
fix the item we want to delete.
- // To do an orderless copy, a copy of the expected result is created.
Results are removed as they are found. This stops
- // the following from returning equal [1 2 2 2] and [1 1 1 2]
-
- // Shortcut.
- if fmt.Sprint(expected) == fmt.Sprint(actual) {
+// compareSingleEqual is the core logical equality function used for Gremlin
Gherkin tests.
+// It avoids using fmt.Sprint which can include memory addresses for pointers
(like Vertices or Edges),
+// leading to false negatives in tests. It performs recursive comparisons for
complex types
+// and handles Gremlin-specific elements by comparing their IDs.
+func compareSingleEqual(actual interface{}, expected interface{}) bool {
+ if actual == nil && expected == nil {
return true
+ } else if actual == nil || expected == nil {
+ return false
}
+
+ actualSet, ok1 := actual.(*gremlingo.SimpleSet)
+ expectedSet, ok2 := expected.(*gremlingo.SimpleSet)
+ if ok1 && ok2 {
+ return compareListEqualsWithoutOrder(expectedSet.ToSlice(),
actualSet.ToSlice())
+ } else if ok1 || ok2 {
+ return false
+ }
+
+ actualPath, ok1 := actual.(*gremlingo.Path)
+ expectedPath, ok2 := expected.(*gremlingo.Path)
+ if ok1 && ok2 {
+ return compareListEqualsWithOrder(expectedPath.Objects,
actualPath.Objects)
+ } else if ok1 || ok2 {
+ return false
+ }
+
+ actualV, ok1 := actual.(*gremlingo.Vertex)
+ expectedV, ok2 := expected.(*gremlingo.Vertex)
+ if ok1 && ok2 {
+ return actualV.Id == expectedV.Id
+ }
+
+ actualE, ok1 := actual.(*gremlingo.Edge)
+ expectedE, ok2 := expected.(*gremlingo.Edge)
+ if ok1 && ok2 {
+ return actualE.Id == expectedE.Id
+ }
+
+ actualVP, ok1 := actual.(*gremlingo.VertexProperty)
+ expectedVP, ok2 := expected.(*gremlingo.VertexProperty)
+ if ok1 && ok2 {
+ return actualVP.Id == expectedVP.Id
+ }
+
+ actualP, ok1 := actual.(*gremlingo.Property)
+ expectedP, ok2 := expected.(*gremlingo.Property)
+ if ok1 && ok2 {
+ return actualP.Key == expectedP.Key &&
compareSingleEqual(actualP.Value, expectedP.Value)
+ }
+
+ switch a := actual.(type) {
+ case map[interface{}]interface{}:
+ e, ok := expected.(map[interface{}]interface{})
+ if !ok {
+ return false
+ }
+ return compareMapEquals(e, a)
+ case []interface{}:
+ e, ok := expected.([]interface{})
+ if !ok {
+ return false
+ }
+ return compareListEqualsWithOrder(e, a)
+ case int, int8, int16, int32, int64, uint, uint8, uint16, uint32,
uint64, float32, float64:
+ return compareNumeric(actual, expected)
+ default:
+ return fmt.Sprint(actual) == fmt.Sprint(expected)
+ }
+}
+
+// compareNumeric normalizes different numeric types to float64 to allow for
comparison
+// between different precisions and representations (e.g. int64 vs float32).
+// It also explicitly handles NaN equality which is required for some Gremlin
tests.
+func compareNumeric(actual interface{}, expected interface{}) bool {
+ var aVal, eVal float64
+ switch a := actual.(type) {
+ case int:
+ aVal = float64(a)
+ case int8:
+ aVal = float64(a)
+ case int16:
+ aVal = float64(a)
+ case int32:
+ aVal = float64(a)
+ case int64:
+ aVal = float64(a)
Review Comment:
i dont want to overcomplicate the testing, but `Set` was vexing Go and after
trying a few more surgical things, i let AI go kinda wild here. maybe it's
doing way more than necessary. should this be revisited more carefully?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
