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>
opensearch-project · Sep 3, 2024 · 8c187d5 · 8c187d5
1 parent 3fc0139
commit 8c187d5
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 35 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
@@ -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:
@@ -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
@@ -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() {