Skip to content

Commit f4b97d0

Browse files
dfa1claude
dfa1
and
claude
committed
test: isolate the subject call in throwing-assertion lambdas (S5778)
Sonar S5778 (81 hits, the top code smell): assertThatThrownBy lambdas that contain more than one possibly-throwing call can't tell which one raised. Hoisting the setup calls (fixture builders, WriteOptions.defaults(), List.of/Map.of args, builder chains) to locals leaves exactly the subject-under-test in the lambda — so an unexpected throw in setup fails clearly instead of masquerading as the asserted exception. First batch: the parse/bounds/writer-policy security tests where that precision matters most — 17 sites across 5 files. Behavior unchanged; all touched suites green. ~64 sites in other test files remain for follow-up batches. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent a150534 commit f4b97d0

5 files changed

Lines changed: 51 additions & 29 deletions

File tree

‎core/src/test/java/io/github/dfa1/vortex/core/model/DTypeStructBuilderTest.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,10 +92,11 @@ void builder_isNotReusable_afterMutation_byField() {
9292
@org.junit.jupiter.params.ParameterizedTest
9393
@org.junit.jupiter.params.provider.ValueSource(strings = {"", " ", " ", "a\nb", "nul\u0000here"})
9494
void field_footgunName_throwsIllegalArgumentException(String name) {
95-
// Given / When / Then — the friendly path enforces the write-side name policy up front:
95+
// Given the friendly path, which enforces the write-side name policy up front:
9696
// blank and control-character names are wire-legal footguns vortex-java refuses to write
97-
org.assertj.core.api.Assertions.assertThatThrownBy(
98-
() -> DType.structBuilder().field(name, DType.I64))
97+
var builder = DType.structBuilder();
98+
// When / Then
99+
org.assertj.core.api.Assertions.assertThatThrownBy(() -> builder.field(name, DType.I64))
99100
.isInstanceOf(IllegalArgumentException.class);
100101
}
101102
}

‎core/src/test/java/io/github/dfa1/vortex/core/model/DTypeStructValidationTest.java‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,23 +17,30 @@ class DTypeStructValidationTest {
1717

1818
@Test
1919
void construct_arityMismatch_throwsIllegalArgumentException() {
20-
// Given / When / Then — two names, one type: the record constructor rejects the desync
21-
assertThatThrownBy(() -> new DType.Struct(List.of("a", "b"), List.of(I64), false))
20+
// Given two names, one type: the record constructor rejects the desync
21+
var names = List.of("a", "b");
22+
var types = List.of(I64);
23+
// When / Then
24+
assertThatThrownBy(() -> new DType.Struct(names, types, false))
2225
.isInstanceOf(IllegalArgumentException.class)
2326
.hasMessageContaining("size mismatch");
2427
}
2528

2629
@Test
2730
void construct_nullNames_throwsNullPointerException() {
28-
// Given / When / Then
29-
assertThatThrownBy(() -> new DType.Struct(null, List.of(I64), false))
31+
// Given a null names list
32+
var types = List.of(I64);
33+
// When / Then
34+
assertThatThrownBy(() -> new DType.Struct(null, types, false))
3035
.isInstanceOf(NullPointerException.class);
3136
}
3237

3338
@Test
3439
void construct_nullTypes_throwsNullPointerException() {
35-
// Given / When / Then
36-
assertThatThrownBy(() -> new DType.Struct(List.of("a"), null, false))
40+
// Given a null types list
41+
var names = List.of("a");
42+
// When / Then
43+
assertThatThrownBy(() -> new DType.Struct(names, null, false))
3744
.isInstanceOf(NullPointerException.class);
3845
}
3946

‎reader/src/test/java/io/github/dfa1/vortex/reader/PostscriptParserDTypeGuardsTest.java‎

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,10 @@ void convertDType_duplicateStructFieldNames_throwsVortexException() {
3030
// Given — a dtype blob whose root struct declares two fields named "dup"
3131
MemorySegment dtype = structDType(new String[]{"dup", "dup"}, 2);
3232

33-
// When / Then
34-
assertThatThrownBy(() -> PostscriptParser.parseBlobs(minimalFooter(), flatLayout(), dtype))
33+
// When / Then — footer/layout fixtures hoisted so only the subject call is in the lambda
34+
MemorySegment footer = minimalFooter();
35+
MemorySegment layout = flatLayout();
36+
assertThatThrownBy(() -> PostscriptParser.parseBlobs(footer, layout, dtype))
3537
.isInstanceOf(VortexException.class)
3638
.hasMessageContaining("duplicate field name in file schema: dup");
3739
}
@@ -41,8 +43,10 @@ void convertDType_structNamesDtypesArityMismatch_throwsVortexException() {
4143
// Given — a dtype blob declaring two field names but only one field type
4244
MemorySegment dtype = structDType(new String[]{"a", "b"}, 1);
4345

44-
// When / Then
45-
assertThatThrownBy(() -> PostscriptParser.parseBlobs(minimalFooter(), flatLayout(), dtype))
46+
// When / Then — footer/layout fixtures hoisted so only the subject call is in the lambda
47+
MemorySegment footer = minimalFooter();
48+
MemorySegment layout = flatLayout();
49+
assertThatThrownBy(() -> PostscriptParser.parseBlobs(footer, layout, dtype))
4650
.isInstanceOf(VortexException.class)
4751
.hasMessageContaining("names/dtypes length mismatch");
4852
}
@@ -54,8 +58,10 @@ void convertDType_blankFieldName_throwsVortexException(String name) {
5458
// produce it) — rejected by policy with a message pointing at the producing pipeline
5559
MemorySegment dtype = structDType(new String[]{name}, 1);
5660

57-
// When / Then
58-
assertThatThrownBy(() -> PostscriptParser.parseBlobs(minimalFooter(), flatLayout(), dtype))
61+
// When / Then — footer/layout fixtures hoisted so only the subject call is in the lambda
62+
MemorySegment footer = minimalFooter();
63+
MemorySegment layout = flatLayout();
64+
assertThatThrownBy(() -> PostscriptParser.parseBlobs(footer, layout, dtype))
5965
.isInstanceOf(VortexException.class)
6066
.hasMessageContaining("invalid field name in file schema")
6167
.hasMessageContaining("blank field name");
@@ -67,8 +73,10 @@ void convertDType_controlCharacterFieldName_throwsVortexException() {
6773
// SQL/CSV/log renderings downstream, so the reader rejects it like the writer does
6874
MemorySegment dtype = structDType(new String[]{"a\nb"}, 1);
6975

70-
// When / Then
71-
assertThatThrownBy(() -> PostscriptParser.parseBlobs(minimalFooter(), flatLayout(), dtype))
76+
// When / Then — footer/layout fixtures hoisted so only the subject call is in the lambda
77+
MemorySegment footer = minimalFooter();
78+
MemorySegment layout = flatLayout();
79+
assertThatThrownBy(() -> PostscriptParser.parseBlobs(footer, layout, dtype))
7280
.isInstanceOf(VortexException.class)
7381
.hasMessageContaining("control character U+000A");
7482
}

‎reader/src/test/java/io/github/dfa1/vortex/reader/SerializedArrayBoundsSecurityTest.java‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ class SerializedArrayBoundsSecurityTest {
2525

2626

2727
private static final DType DTYPE = DType.I32;
28+
private static final List<String> FLAT_SPEC = List.of("vortex.flat");
2829

2930
private final SerializedArrayDecoder sut = new SerializedArrayDecoder(ReadRegistry.empty());
3031

@@ -35,7 +36,7 @@ void segmentSmallerThanLengthField_throwsVortexException() {
3536
MemorySegment seg = arena.allocate(2);
3637

3738
// When / Then the length-field read is bounds-checked, not left to crash get()
38-
assertThatThrownBy(() -> sut.decode(seg, List.of("vortex.flat"), DTYPE, 1, arena))
39+
assertThatThrownBy(() -> sut.decode(seg, FLAT_SPEC, DTYPE, 1, arena))
3940
.isInstanceOf(VortexException.class);
4041
}
4142
}
@@ -49,7 +50,7 @@ void declaredFbLenLargerThanSegment_throwsVortexException() {
4950
seg.set(VortexFormat.LE_INT, 12, 1_000_000);
5051

5152
// When / Then
52-
assertThatThrownBy(() -> sut.decode(seg, List.of("vortex.flat"), DTYPE, 1, arena))
53+
assertThatThrownBy(() -> sut.decode(seg, FLAT_SPEC, DTYPE, 1, arena))
5354
.isInstanceOf(VortexException.class);
5455
}
5556
}
@@ -62,7 +63,7 @@ void negativeFbLen_throwsVortexException() {
6263
seg.set(VortexFormat.LE_INT, 12, -1);
6364

6465
// When / Then the negative length is rejected (checkRange len < 0)
65-
assertThatThrownBy(() -> sut.decode(seg, List.of("vortex.flat"), DTYPE, 1, arena))
66+
assertThatThrownBy(() -> sut.decode(seg, FLAT_SPEC, DTYPE, 1, arena))
6667
.isInstanceOf(VortexException.class);
6768
}
6869
}
@@ -78,7 +79,7 @@ void bufferDescriptorLengthPastSegment_throwsVortexException() {
7879
seg.set(VortexFormat.LE_INT, fb.length, fb.length);
7980

8081
// When / Then the buffer slice is bounds-checked before asSlice
81-
assertThatThrownBy(() -> sut.decode(seg, List.of("vortex.flat"), DTYPE, 1, arena))
82+
assertThatThrownBy(() -> sut.decode(seg, FLAT_SPEC, DTYPE, 1, arena))
8283
.isInstanceOf(VortexException.class);
8384
}
8485
}

‎writer/src/test/java/io/github/dfa1/vortex/writer/VortexWriterTest.java‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -91,9 +91,10 @@ void create_duplicateFieldNames_throwsIllegalArgumentException(@TempDir Path tmp
9191
false);
9292
Path file = tmp.resolve("dup.vtx");
9393

94-
// When / Then
94+
// When / Then — options hoisted so only the subject call is in the lambda
95+
WriteOptions opts = WriteOptions.defaults();
9596
try (var ch = FileChannel.open(file, StandardOpenOption.CREATE, StandardOpenOption.WRITE)) {
96-
assertThatThrownBy(() -> VortexWriter.create(ch, schema, WriteOptions.defaults()))
97+
assertThatThrownBy(() -> VortexWriter.create(ch, schema, opts))
9798
.isInstanceOf(IllegalArgumentException.class)
9899
.hasMessage("duplicate field name: dup");
99100
}
@@ -110,9 +111,10 @@ void create_fieldNameWithNulByte_throwsIllegalArgumentException(@TempDir Path tm
110111
false);
111112
Path file = tmp.resolve("nul.vtx");
112113

113-
// When / Then
114+
// When / Then — options hoisted so only the subject call is in the lambda
115+
WriteOptions opts = WriteOptions.defaults();
114116
try (var ch = FileChannel.open(file, StandardOpenOption.CREATE, StandardOpenOption.WRITE)) {
115-
assertThatThrownBy(() -> VortexWriter.create(ch, schema, WriteOptions.defaults()))
117+
assertThatThrownBy(() -> VortexWriter.create(ch, schema, opts))
116118
.isInstanceOf(IllegalArgumentException.class)
117119
.hasMessageContaining("U+0000");
118120
}
@@ -130,9 +132,10 @@ void create_footgunFieldName_throwsIllegalArgumentException(String name, @TempDi
130132
false);
131133
Path file = tmp.resolve("footgun.vtx");
132134

133-
// When / Then
135+
// When / Then — options hoisted so only the subject call is in the lambda
136+
WriteOptions opts = WriteOptions.defaults();
134137
try (var ch = FileChannel.open(file, StandardOpenOption.CREATE, StandardOpenOption.WRITE)) {
135-
assertThatThrownBy(() -> VortexWriter.create(ch, schema, WriteOptions.defaults()))
138+
assertThatThrownBy(() -> VortexWriter.create(ch, schema, opts))
136139
.isInstanceOf(IllegalArgumentException.class);
137140
}
138141
}
@@ -235,7 +238,8 @@ void writeChunk_map_nonNullablePrimitive_rejectsBoxedArray(@TempDir Path tmp) th
235238
try (var ch = FileChannel.open(file, StandardOpenOption.CREATE, StandardOpenOption.WRITE);
236239
var sut = VortexWriter.create(ch, SCHEMA, WriteOptions.defaults())) {
237240
// When / Then
238-
assertThatThrownBy(() -> sut.writeChunk(Map.of("id", new Long[]{1L, 2L})))
241+
Map<String, Object> boxedColumn = Map.of("id", new Long[]{1L, 2L});
242+
assertThatThrownBy(() -> sut.writeChunk(boxedColumn))
239243
.isInstanceOf(IllegalArgumentException.class)
240244
.hasMessageContaining("non-nullable")
241245
.hasMessageContaining("id");
@@ -407,7 +411,8 @@ void writeChunk_missingColumn_throwsIllegalArgument(@TempDir Path tmp) throws IO
407411
try (var ch = FileChannel.open(file, StandardOpenOption.CREATE, StandardOpenOption.WRITE);
408412
var sut = VortexWriter.create(ch, SCHEMA, WriteOptions.defaults())) {
409413
// When / Then
410-
assertThatThrownBy(() -> sut.writeChunk(Map.of("id", new long[]{1L})))
414+
Map<String, Object> partialColumns = Map.of("id", new long[]{1L});
415+
assertThatThrownBy(() -> sut.writeChunk(partialColumns))
411416
.isInstanceOf(IllegalArgumentException.class)
412417
.hasMessageContaining("missing column: value");
413418
}

0 commit comments

Comments
 (0)