Skip to content

Commit 7b2c610

Browse files
whitewhite
authored andcommitted
Add Builder validation and enhance test coverage
- PythonEmbed.Options.Builder: validate timeoutMs, maxCodeLength, startupTimeoutMs > 0 - PythonEmbedPoolTest: add pool exhaustion tests (CallerRunsPolicy) and Builder validation tests - PythonValueTest: add null-safety tests for all typed accessors (11 tests) - PythonEmbedIntegrationTest: add eval_codeExceedingMaxLength_throws integration test
1 parent eb53e94 commit 7b2c610

4 files changed

Lines changed: 190 additions & 0 deletions

File tree

python-embed-runtime/src/main/java/io/github/howtis/pythonembed/PythonEmbed.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1375,7 +1375,23 @@ public Builder onAfterClose(CloseHook hook) {
13751375
return this;
13761376
}
13771377

1378+
/**
1379+
* Builds an {@link Options} instance from the builder's current values.
1380+
*
1381+
* @return a new {@link Options} instance
1382+
* @throws IllegalArgumentException if {@code timeoutMs <= 0},
1383+
* {@code maxCodeLength <= 0}, or {@code startupTimeoutMs <= 0}
1384+
*/
13781385
public Options build() {
1386+
if (timeoutMs <= 0) {
1387+
throw new IllegalArgumentException("timeoutMs must be positive");
1388+
}
1389+
if (maxCodeLength <= 0) {
1390+
throw new IllegalArgumentException("maxCodeLength must be positive");
1391+
}
1392+
if (startupTimeoutMs <= 0) {
1393+
throw new IllegalArgumentException("startupTimeoutMs must be positive");
1394+
}
13791395
return new Options(timeoutMs,
13801396
maxCodeLength, startupTimeoutMs, pythonExecutable,
13811397
List.copyOf(warmupScripts), lenientWarmup,

python-embed-runtime/src/test/java/io/github/howtis/pythonembed/PythonEmbedIntegrationTest.java

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,29 @@ void eval_nameError_throws() {
106106
assertTrue(ex.getMessage().contains("NameError"));
107107
}
108108

109+
@Test
110+
@Timeout(value = 30, unit = TimeUnit.SECONDS)
111+
void eval_codeExceedingMaxLength_throws() {
112+
PythonEmbed.Options strictOpts = PythonEmbed.Options.builder()
113+
.timeoutMs(60_000)
114+
.maxCodeLength(5)
115+
.startupTimeoutMs(30_000)
116+
.venvPath(Path.of("build", "python-venv"))
117+
.build();
118+
try (PythonEmbed strictPy = PythonEmbed.create(strictOpts)) {
119+
// Code is 7 chars (> 5) but the frame is well under 50 bytes
120+
String longCode = "1+2+3+4";
121+
PythonExecutionException ex = assertThrows(
122+
PythonExecutionException.class,
123+
() -> strictPy.eval(longCode)
124+
);
125+
assertTrue(
126+
ex.getMessage().contains("maximum length")
127+
|| ex.getMessage().contains("exceeds"),
128+
"Expected max code length error, got: " + ex.getMessage());
129+
}
130+
}
131+
109132
@Test
110133
@Timeout(value = 3, unit = TimeUnit.SECONDS)
111134
void exec_multiline_functionDef() {

python-embed-runtime/src/test/java/io/github/howtis/pythonembed/PythonEmbedPoolTest.java

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,53 @@ void eval_nameError_propagates() {
186186
assertTrue(ex.getCause().getMessage().contains("NameError"));
187187
}
188188

189+
// ------------------------------------------------------------------
190+
// Pool exhaustion (CallerRunsPolicy backpressure)
191+
// ------------------------------------------------------------------
192+
193+
@Test
194+
@Timeout(value = 30, unit = TimeUnit.SECONDS)
195+
void exhaustion_tasksCompleteWithoutRejection() throws Exception {
196+
PythonEmbedPool smallPool = PythonEmbedPool.builder().minPool(1).maxPool(1).build();
197+
try {
198+
// Submit more tasks than maxPool (1). CallerRunsPolicy ensures
199+
// no RejectedExecutionException -- all tasks complete.
200+
int taskCount = 5;
201+
@SuppressWarnings("unchecked")
202+
CompletableFuture<PythonValue>[] futures = new CompletableFuture[taskCount];
203+
for (int i = 0; i < taskCount; i++) {
204+
futures[i] = smallPool.eval(String.valueOf(i * 10));
205+
}
206+
for (int i = 0; i < taskCount; i++) {
207+
assertEquals(i * 10, futures[i].get(5, TimeUnit.SECONDS).asInt());
208+
}
209+
} finally {
210+
smallPool.close();
211+
}
212+
}
213+
214+
@Test
215+
@Timeout(value = 30, unit = TimeUnit.SECONDS)
216+
void exhaustion_longRunningTasks_stillComplete() throws Exception {
217+
PythonEmbedPool smallPool = PythonEmbedPool.builder().minPool(1).maxPool(2).build();
218+
try {
219+
// Saturate with two long-ish tasks. The third task exercises
220+
// CallerRunsPolicy: it runs on the caller thread once executor
221+
// threads and semaphore permits are all taken.
222+
CompletableFuture<PythonValue> slow1 = smallPool.eval("__import__('time').sleep(1) or 1");
223+
CompletableFuture<PythonValue> slow2 = smallPool.eval("__import__('time').sleep(1) or 2");
224+
225+
// Submit one more -- CallerRunsPolicy will eventually handle it
226+
CompletableFuture<PythonValue> extra = smallPool.eval("42");
227+
228+
assertEquals(1, slow1.get(10, TimeUnit.SECONDS).asInt());
229+
assertEquals(2, slow2.get(10, TimeUnit.SECONDS).asInt());
230+
assertEquals(42, extra.get(10, TimeUnit.SECONDS).asInt());
231+
} finally {
232+
smallPool.close();
233+
}
234+
}
235+
189236
// ------------------------------------------------------------------
190237
// Stream
191238
// ------------------------------------------------------------------
@@ -584,6 +631,34 @@ void maxPool_mustBeAtLeastMinPool() {
584631
assertThrows(IllegalArgumentException.class, () -> PythonEmbedPool.builder().minPool(3).maxPool(2).build());
585632
}
586633

634+
// ------------------------------------------------------------------
635+
// Options.Builder validation
636+
// ------------------------------------------------------------------
637+
638+
@Test
639+
void options_timeoutMs_mustBePositive() {
640+
assertThrows(IllegalArgumentException.class,
641+
() -> PythonEmbed.Options.builder().timeoutMs(0).build());
642+
assertThrows(IllegalArgumentException.class,
643+
() -> PythonEmbed.Options.builder().timeoutMs(-1).build());
644+
}
645+
646+
@Test
647+
void options_maxCodeLength_mustBePositive() {
648+
assertThrows(IllegalArgumentException.class,
649+
() -> PythonEmbed.Options.builder().maxCodeLength(0).build());
650+
assertThrows(IllegalArgumentException.class,
651+
() -> PythonEmbed.Options.builder().maxCodeLength(-1).build());
652+
}
653+
654+
@Test
655+
void options_startupTimeoutMs_mustBePositive() {
656+
assertThrows(IllegalArgumentException.class,
657+
() -> PythonEmbed.Options.builder().startupTimeoutMs(0).build());
658+
assertThrows(IllegalArgumentException.class,
659+
() -> PythonEmbed.Options.builder().startupTimeoutMs(-1).build());
660+
}
661+
587662
// ------------------------------------------------------------------
588663
// Scale-down (cleanupIdleInstances)
589664
// ------------------------------------------------------------------

python-embed-runtime/src/test/java/io/github/howtis/pythonembed/PythonValueTest.java

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,4 +268,80 @@ void typedMap_wrongType_throws() {
268268
assertThrows(ClassCastException.class, v::asIntMap);
269269
assertThrows(ClassCastException.class, v::asLongMap);
270270
}
271+
272+
// ---- null value safety ----
273+
274+
@Test
275+
void isNull_returnsTrue_whenRawIsNull() {
276+
PythonValue v = PythonValue.of(null);
277+
assertTrue(v.isNull());
278+
assertNull(v.raw());
279+
}
280+
281+
@Test
282+
void asInt_onNull_throwsIllegalStateException() {
283+
PythonValue v = PythonValue.of(null);
284+
assertTrue(v.isNull());
285+
assertThrows(IllegalStateException.class, v::asInt);
286+
}
287+
288+
@Test
289+
void asLong_onNull_throwsIllegalStateException() {
290+
PythonValue v = PythonValue.of(null);
291+
assertThrows(IllegalStateException.class, v::asLong);
292+
}
293+
294+
@Test
295+
void asDouble_onNull_throwsIllegalStateException() {
296+
PythonValue v = PythonValue.of(null);
297+
assertThrows(IllegalStateException.class, v::asDouble);
298+
}
299+
300+
@Test
301+
void asBoolean_onNull_throwsIllegalStateException() {
302+
PythonValue v = PythonValue.of(null);
303+
assertThrows(IllegalStateException.class, v::asBoolean);
304+
}
305+
306+
@Test
307+
void asString_onNull_returnsNullLiteral() {
308+
PythonValue v = PythonValue.of(null);
309+
assertEquals("null", v.asString());
310+
}
311+
312+
@Test
313+
void asList_onNull_throwsIllegalStateException() {
314+
PythonValue v = PythonValue.of(null);
315+
assertThrows(IllegalStateException.class, () -> v.asList(Integer.class));
316+
}
317+
318+
@Test
319+
void asList_noArg_onNull_throwsIllegalStateException() {
320+
PythonValue v = PythonValue.of(null);
321+
assertThrows(IllegalStateException.class, v::asList);
322+
}
323+
324+
@Test
325+
void asMap_onNull_throwsIllegalStateException() {
326+
PythonValue v = PythonValue.of(null);
327+
assertThrows(IllegalStateException.class, () -> v.asMap(String.class, Double.class));
328+
}
329+
330+
@Test
331+
void asMap_noArg_onNull_throwsIllegalStateException() {
332+
PythonValue v = PythonValue.of(null);
333+
assertThrows(IllegalStateException.class, v::asMap);
334+
}
335+
336+
@Test
337+
void asBytes_onNull_throwsIllegalStateException() {
338+
PythonValue v = PythonValue.of(null);
339+
assertThrows(IllegalStateException.class, v::asBytes);
340+
}
341+
342+
@Test
343+
void toString_onNull_showsNull() {
344+
PythonValue v = PythonValue.of(null);
345+
assertEquals("PythonValue{null}", v.toString());
346+
}
271347
}

0 commit comments

Comments
 (0)