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 · Aug 31, 2024 · 8fecdc5 · 8fecdc5
1 parent 2224d48
commit 8fecdc5
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 25 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,14 +181,16 @@ public void compare(String field, boolean hadKey, @Nullable Object actual, Objec
             field(field, "same [" + expected + "]");
             return;
         }
+        String expectedClass = expected == null ? "null object" : expected.getClass().getSimpleName();
+        String actualClass = actual == null ? "null object" : actual.getClass().getSimpleName();
         field(
             field,
             "expected "
-                + expected.getClass().getSimpleName()
+                + expectedClass
                 + " ["
                 + expected
                 + "] but was "
-                + actual.getClass().getSimpleName()
+                + actualClass
                 + " ["
                 + actual
                 + "]"