From 1983ac0a3b03a447812208688db6dbd425afd09e Mon Sep 17 00:00:00 2001
From: Michael Froh <froh@amazon.com>
Date: Tue, 3 Sep 2024 15:14:40 -0700
Subject: [PATCH] Fix flaky test in 91_flat_object_null_value.yml (#15545)

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() {