Skip to content

Commit b9c570d

Browse files
msfrohakolarkunnu
authored andcommitted
Fix flaky test in 91_flat_object_null_value.yml (opensearch-project#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 <[email protected]>
1 parent 85add43 commit b9c570d

File tree

2 files changed

+52
-36
lines changed

2 files changed

+52
-36
lines changed

rest-api-spec/src/main/resources/rest-api-spec/test/index/91_flat_object_null_value.yml

Lines changed: 49 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,15 @@ setup:
1616
properties:
1717
record:
1818
type: "flat_object"
19+
order:
20+
type: "integer"
1921
- do:
2022
index:
2123
index: flat_object_null_value
2224
id: 1
2325
body: {
24-
"record": null
26+
"record": null,
27+
"order" : 1
2528
}
2629

2730
- do:
@@ -31,7 +34,8 @@ setup:
3134
body: {
3235
"record": {
3336
"name": null
34-
}
37+
},
38+
"order" : 2
3539
}
3640

3741
- do:
@@ -43,7 +47,8 @@ setup:
4347
"name": null,
4448
"age":"5",
4549
"name1": null
46-
}
50+
},
51+
"order" : 3
4752
}
4853

4954
- do:
@@ -60,7 +65,8 @@ setup:
6065
}
6166
}
6267
]
63-
}
68+
},
69+
"order" : 4
6470
}
6571

6672
- do:
@@ -77,7 +83,8 @@ setup:
7783
},
7884
null
7985
]
80-
}
86+
},
87+
"order" : 5
8188
}
8289

8390
- do:
@@ -97,7 +104,8 @@ setup:
97104
}
98105
}
99106
]
100-
}
107+
},
108+
"order" : 6
101109
}
102110

103111
- do:
@@ -108,7 +116,8 @@ setup:
108116
"record": {
109117
"name": null,
110118
"age":"3"
111-
}
119+
},
120+
"order" : 7
112121
}
113122

114123
- do:
@@ -119,7 +128,8 @@ setup:
119128
"record": {
120129
"age":"3",
121130
"name": null
122-
}
131+
},
132+
"order" : 8
123133
}
124134

125135
- do:
@@ -133,7 +143,8 @@ setup:
133143
3
134144
],
135145
"age": 4
136-
}
146+
},
147+
"order" : 9
137148
}
138149

139150
- do:
@@ -147,7 +158,8 @@ setup:
147158
null,
148159
3
149160
]
150-
}
161+
},
162+
"order" : 10
151163
}
152164

153165
- do:
@@ -157,7 +169,8 @@ setup:
157169
body: {
158170
"record": {
159171
"name": null
160-
}
172+
},
173+
"order": 11
161174
}
162175

163176
- do:
@@ -171,7 +184,8 @@ setup:
171184
null
172185
]
173186
}
174-
}
187+
},
188+
"order": 12
175189
}
176190

177191
- do:
@@ -183,7 +197,8 @@ setup:
183197
"labels": [
184198
null
185199
]
186-
}
200+
},
201+
"order": 13
187202
}
188203

189204
- do:
@@ -198,7 +213,8 @@ setup:
198213
null
199214
]
200215
}
201-
}
216+
},
217+
"order": 14
202218
}
203219

204220
- do:
@@ -211,7 +227,8 @@ setup:
211227
"labels": [
212228
null
213229
]
214-
}
230+
},
231+
"order": 15
215232
}
216233

217234
- do:
@@ -224,7 +241,8 @@ setup:
224241
null
225242
],
226243
"age": "4"
227-
}
244+
},
245+
"order": 16
228246
}
229247

230248
- do:
@@ -239,7 +257,8 @@ setup:
239257
"dsdsdsd"
240258
]
241259
}
242-
}
260+
},
261+
"order": 17
243262
}
244263

245264
- do:
@@ -253,7 +272,8 @@ setup:
253272
"name2": null
254273
}
255274
}
256-
}
275+
},
276+
"order": 18
257277
}
258278

259279
- do:
@@ -271,7 +291,8 @@ setup:
271291
]
272292
]
273293
}
274-
}
294+
},
295+
"order": 19
275296
}
276297

277298
- do:
@@ -299,7 +320,7 @@ teardown:
299320
- is_true: flat_object_null_value.mappings
300321
- match: { flat_object_null_value.mappings.properties.record.type: flat_object }
301322
# https://github.com/opensearch-project/OpenSearch/tree/main/rest-api-spec/src/main/resources/rest-api-spec/test#length
302-
- length: { flat_object_null_value.mappings.properties: 1 }
323+
- length: { flat_object_null_value.mappings.properties: 2 }
303324

304325

305326
---
@@ -328,7 +349,8 @@ teardown:
328349
size: 30,
329350
query: {
330351
exists: { "field": "record" }
331-
}
352+
},
353+
sort: [{ order: asc}]
332354
}
333355

334356
- length: { hits.hits: 12 }
@@ -352,7 +374,8 @@ teardown:
352374
_source: true,
353375
query: {
354376
exists: { "field": "record.d" }
355-
}
377+
},
378+
sort: [{ order: asc}]
356379
}
357380

358381
- length: { hits.hits: 3 }
@@ -367,7 +390,8 @@ teardown:
367390
_source: true,
368391
query: {
369392
term: { record: "dsdsdsd" }
370-
}
393+
},
394+
sort: [{ order: asc}]
371395
}
372396

373397
- length: { hits.hits: 2 }
@@ -381,7 +405,8 @@ teardown:
381405
_source: true,
382406
query: {
383407
term: { record.name.name1: "dsdsdsd" }
384-
}
408+
},
409+
sort: [{ order: asc}]
385410
}
386411

387412
- length: { hits.hits: 2 }

test/framework/src/main/java/org/opensearch/test/NotEqualMessageBuilder.java

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -181,18 +181,9 @@ public void compare(String field, boolean hadKey, @Nullable Object actual, Objec
181181
field(field, "same [" + expected + "]");
182182
return;
183183
}
184-
field(
185-
field,
186-
"expected "
187-
+ expected.getClass().getSimpleName()
188-
+ " ["
189-
+ expected
190-
+ "] but was "
191-
+ actual.getClass().getSimpleName()
192-
+ " ["
193-
+ actual
194-
+ "]"
195-
);
184+
String expectedClass = expected == null ? "null object" : expected.getClass().getSimpleName();
185+
String actualClass = actual == null ? "null object" : actual.getClass().getSimpleName();
186+
field(field, "expected " + expectedClass + " [" + expected + "] but was " + actualClass + " [" + actual + "]");
196187
}
197188

198189
private void indent() {

0 commit comments

Comments
 (0)