From ba7a34ae0d7a3eb65e28d769052eefbaf23ed2d4 Mon Sep 17 00:00:00 2001
From: Michael Froh <froh@amazon.com>
Date: Sat, 31 Aug 2024 00:01:12 +0000
Subject: [PATCH] Fix flaky test in 91_flat_object_null_value.yml

This test assumed that the order of returned hits will match the order
of insertion. That's not generally true, especially if there was a
flush partway through, so documents end up in different segments.

This fixes it by explicitly sorting the returned documents to
guarantee that they come back in the correct order.

Also, we were getting a NPE when trying to output the failure message
because the expected value was intentionally null. I fixed that too.

Signed-off-by: Michael Froh <froh@amazon.com>
---
 .../test/index/91_flat_object_null_value.yml  | 73 +++++++++++++------
 .../test/NotEqualMessageBuilder.java          | 15 +---
 2 files changed, 52 insertions(+), 36 deletions(-)

diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/index/91_flat_object_null_value.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/index/91_flat_object_null_value.yml
index 98abd58a54e4b..716b6fb51cb43 100644
--- a/rest-api-spec/src/main/resources/rest-api-spec/test/index/91_flat_object_null_value.yml
+++ b/rest-api-spec/src/main/resources/rest-api-spec/test/index/91_flat_object_null_value.yml
@@ -16,12 +16,15 @@ setup:
             properties:
               record:
                 type: "flat_object"
+              order:
+                type: "integer"
   - do:
       index:
         index: flat_object_null_value
         id: 1
         body: {
-          "record": null
+          "record": null,
+          "order" : 1
         }
 
   - do:
@@ -31,7 +34,8 @@ setup:
         body: {
           "record": {
             "name": null
-          }
+          },
+          "order" : 2
         }
 
   - do:
@@ -43,7 +47,8 @@ setup:
             "name": null,
             "age":"5",
             "name1": null
-          }
+          },
+          "order" : 3
         }
 
   - do:
@@ -60,7 +65,8 @@ setup:
                 }
               }
             ]
-          }
+          },
+          "order" : 4
         }
 
   - do:
@@ -77,7 +83,8 @@ setup:
               },
               null
             ]
-          }
+          },
+          "order" : 5
         }
 
   - do:
@@ -97,7 +104,8 @@ setup:
                 }
               }
             ]
-          }
+          },
+          "order" : 6
         }
 
   - do:
@@ -108,7 +116,8 @@ setup:
           "record": {
             "name": null,
             "age":"3"
-          }
+          },
+          "order" : 7
         }
 
   - do:
@@ -119,7 +128,8 @@ setup:
           "record": {
             "age":"3",
             "name": null
-          }
+          },
+          "order" : 8
         }
 
   - do:
@@ -133,7 +143,8 @@ setup:
               3
             ],
             "age": 4
-          }
+          },
+          "order" : 9
         }
 
   - do:
@@ -147,7 +158,8 @@ setup:
               null,
               3
             ]
-          }
+          },
+          "order" : 10
         }
 
   - do:
@@ -157,7 +169,8 @@ setup:
         body: {
           "record": {
             "name": null
-          }
+          },
+          "order": 11
         }
 
   - do:
@@ -171,7 +184,8 @@ setup:
                 null
               ]
             }
-          }
+          },
+          "order": 12
         }
 
   - do:
@@ -183,7 +197,8 @@ setup:
             "labels": [
               null
             ]
-          }
+          },
+          "order": 13
         }
 
   - do:
@@ -198,7 +213,8 @@ setup:
                 null
               ]
             }
-          }
+          },
+          "order": 14
         }
 
   - do:
@@ -211,7 +227,8 @@ setup:
             "labels": [
               null
             ]
-          }
+          },
+          "order": 15
         }
 
   - do:
@@ -224,7 +241,8 @@ setup:
               null
             ],
             "age": "4"
-          }
+          },
+          "order": 16
         }
 
   - do:
@@ -239,7 +257,8 @@ setup:
                 "dsdsdsd"
               ]
             }
-          }
+          },
+          "order": 17
         }
 
   - do:
@@ -253,7 +272,8 @@ setup:
                 "name2": null
               }
             }
-          }
+          },
+          "order": 18
         }
 
   - do:
@@ -271,7 +291,8 @@ setup:
                 ]
               ]
             }
-          }
+          },
+          "order": 19
         }
 
   - do:
@@ -299,7 +320,7 @@ teardown:
   - is_true: flat_object_null_value.mappings
   - match: { flat_object_null_value.mappings.properties.record.type: flat_object }
   # https://github.com/opensearch-project/OpenSearch/tree/main/rest-api-spec/src/main/resources/rest-api-spec/test#length
-  - length: { flat_object_null_value.mappings.properties: 1 }
+  - length: { flat_object_null_value.mappings.properties: 2 }
 
 
 ---
@@ -328,7 +349,8 @@ teardown:
           size: 30,
           query: {
             exists: { "field": "record" }
-          }
+          },
+          sort: [{ order: asc}]
         }
 
   - length: { hits.hits: 12 }
@@ -352,7 +374,8 @@ teardown:
           _source: true,
           query: {
             exists: { "field": "record.d" }
-          }
+          },
+          sort: [{ order: asc}]
         }
 
   - length: { hits.hits: 3 }
@@ -367,7 +390,8 @@ teardown:
           _source: true,
           query: {
             term: { record: "dsdsdsd" }
-          }
+          },
+          sort: [{ order: asc}]
         }
 
   - length: { hits.hits: 2 }
@@ -381,7 +405,8 @@ teardown:
           _source: true,
           query: {
             term: { record.name.name1: "dsdsdsd" }
-          }
+          },
+          sort: [{ order: asc}]
         }
 
   - length: { hits.hits: 2 }
diff --git a/test/framework/src/main/java/org/opensearch/test/NotEqualMessageBuilder.java b/test/framework/src/main/java/org/opensearch/test/NotEqualMessageBuilder.java
index a70e3f15a4bf5..9524b76f255a6 100644
--- a/test/framework/src/main/java/org/opensearch/test/NotEqualMessageBuilder.java
+++ b/test/framework/src/main/java/org/opensearch/test/NotEqualMessageBuilder.java
@@ -181,18 +181,9 @@ public void compare(String field, boolean hadKey, @Nullable Object actual, Objec
             field(field, "same [" + expected + "]");
             return;
         }
-        field(
-            field,
-            "expected "
-                + expected.getClass().getSimpleName()
-                + " ["
-                + expected
-                + "] but was "
-                + actual.getClass().getSimpleName()
-                + " ["
-                + actual
-                + "]"
-        );
+        String expectedClass = expected == null ? "null object" : expected.getClass().getSimpleName();
+        String actualClass = actual == null ? "null object" : actual.getClass().getSimpleName();
+        field(field, "expected " + expectedClass + " [" + expected + "] but was " + actualClass + " [" + actual + "]");
     }
 
     private void indent() {