Skip to content

Commit 23ac017

Browse files
petros-double-test1claude
authored andcommitted
[NA] [DOCS] docs: warn against bare hasSize in collection assertions
hasSize alone asserts a count and nothing about identity, so a bug that preserves the count passes. Called out on #7966, where both new de-duplication tests checked stored.size() and left the row identities unasserted -- the count is exactly what a de-dup bug is most likely to get right. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 98b8482 commit 23ac017

1 file changed

Lines changed: 25 additions & 0 deletions

File tree

.agents/skills/opik-backend/testing.md

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,31 @@ assertThat(actual)
284284
`containsExactly` asserts size and content together — `hasSize` plus per-index checks does not,
285285
and lets an extra element through.
286286

287+
`hasSize` **on its own** is weaker still: it asserts a count and nothing about identity, so any
288+
bug that preserves the count passes. This bites hardest on de-duplication, merge, and upsert
289+
tests, where the count is exactly the thing a bug is most likely to keep right:
290+
291+
```java
292+
// ❌ BAD - passes if the wrong revision survived, or if the duplicate was kept
293+
// and the distinct row dropped. Both keep the size at 2.
294+
assertThat(stored).hasSize(2);
295+
assertThat(version.itemsTotal()).isEqualTo(stored.size());
296+
297+
// ✅ GOOD - names the rows that must survive, so a wrong-winner bug fails
298+
assertThat(stored)
299+
.usingRecursiveFieldByFieldElementComparatorIgnoringFields(IGNORED_FIELDS_DATA_ITEM)
300+
.containsExactlyInAnyOrder(winningDuplicate, distinctItem);
301+
assertThat(version.itemsTotal()).isEqualTo(stored.size());
302+
```
303+
304+
Asserting a derived counter against `stored.size()` is good — it ties the counter to reality
305+
rather than to a literal — but it is only as strong as the assertion on `stored` itself. Pin the
306+
contents first, then tie the counter to them.
307+
308+
Reuse the shared ignore-field constants (`IGNORED_FIELDS_DATA_ITEM` and friends) rather than
309+
declaring a local list: they already encode which server-generated fields are not part of the
310+
contract, and a local copy silently drifts from them.
311+
287312
These `containsExactly*` variants compare elements with the element type's own `equals`, which is
288313
what you want for exact-valued models. When the elements carry `BigDecimal` or `double`, the same
289314
exception that justifies a comparator on a single object applies per element — otherwise a

0 commit comments

Comments
 (0)