Skip to content

Commit cd6affc

Browse files
dougqhdougqh
committed
More end-to-end listener tests
- added tests for temporary file creation - exposed a problem with TestLibraryLoadingListener errors getting swallowed in CompositeLibraryLoadingListener - adding failure tracking into TestLibraryLoadingListener that is checked in assertDone
1 parent 01ef204 commit cd6affc

File tree

6 files changed

+80
-37
lines changed

6 files changed

+80
-37
lines changed

‎components/native-loader/src/main/java/datadog/nativeloader/CompositeLibraryLoadingListener.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,4 +125,9 @@ public CompositeLibraryLoadingListener join(LibraryLoadingListener... listeners)
125125
combinedListeners.addAll(Arrays.asList(listeners));
126126
return new CompositeLibraryLoadingListener(combinedListeners);
127127
}
128+
129+
@Override
130+
public String toString() {
131+
return this.getClass().getSimpleName() + ":" + this.listeners.toString();
132+
}
128133
}

‎components/native-loader/src/main/java/datadog/nativeloader/LibFile.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,8 +118,8 @@ public final String getAbsolutePath() {
118118
@Override
119119
public void close() {
120120
if (this.needsCleanup) {
121-
boolean done = NativeLoader.delete(this.optionalFile);
122-
if (done) {
121+
boolean deleted = NativeLoader.delete(this.optionalFile);
122+
if (deleted) {
123123
this.listeners.onTempFileCleanup(
124124
this.platformSpec, this.optionalComponent, this.libName, this.optionalFile.toPath());
125125
}

‎components/native-loader/src/main/java/datadog/nativeloader/NativeLoader.java‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -331,35 +331,40 @@ private LibFile resolveDynamicImpl(
331331
}
332332

333333
allListeners.onResolveDynamic(platformSpec, optionalComponent, libName, isPreloaded, url);
334-
return this.toLibFile(platformSpec, optionalComponent, libName, url, allListeners);
334+
return toLibFile(this.tempDir, platformSpec, optionalComponent, libName, url, allListeners);
335335
}
336336

337-
private LibFile toLibFile(
338-
PlatformSpec platformSpec,
337+
private static LibFile toLibFile(
338+
Path tempDir,
339+
PlatformSpec platformSpec,
339340
String optionalComponent,
340341
String libName,
341342
URL url,
342-
SafeLibraryLoadingListener listeners)
343+
SafeLibraryLoadingListener allListeners)
343344
throws LibraryLoadException {
344345
if (url.getProtocol().equals("file")) {
345346
return LibFile.fromFile(
346-
platformSpec, optionalComponent, libName, new File(url.getPath()), listeners);
347+
platformSpec, optionalComponent, libName, new File(url.getPath()), allListeners);
347348
} else {
348349
String libExt = PathUtils.dynamicLibExtension(platformSpec);
349350

350351
Path tempFile = null;
351352
try {
352-
tempFile = TempFileHelper.createTempFile(this.tempDir, libName, libExt);
353+
tempFile = TempFileHelper.createTempFile(tempDir, libName, libExt);
353354

354355
try (InputStream in = url.openStream()) {
355356
Files.copy(in, tempFile, StandardCopyOption.REPLACE_EXISTING);
356357
}
357358

359+
allListeners.onTempFileCreated(platformSpec, optionalComponent, libName, tempFile);
360+
358361
return LibFile.fromTempFile(
359-
platformSpec, optionalComponent, libName, tempFile.toFile(), listeners);
362+
platformSpec, optionalComponent, libName, tempFile.toFile(), allListeners);
363+
360364
} catch (Throwable t) {
361-
listeners.onTempFileCreationFailure(
362-
platformSpec, optionalComponent, libName, this.tempDir, libExt, tempFile, t);
365+
allListeners.onTempFileCreationFailure(
366+
platformSpec, optionalComponent, libName, tempDir, libExt, tempFile, t);
367+
363368
throw new LibraryLoadException(libName, t);
364369
}
365370
}

‎components/native-loader/src/test/java/datadog/nativeloader/CompositeLibraryLoadingListenerTest.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,7 @@ public void onLoad() {
5050
TestLibraryLoadingListener listener2 = listener1.copy();
5151

5252
listeners(listener1, listener2)
53-
.onResolveDynamicFailure(
54-
PlatformSpec.defaultPlatformSpec(), null, "foo", new LibraryLoadException("foo"));
53+
.onLoad(PlatformSpec.defaultPlatformSpec(), null, "foo", false, null);
5554

5655
listener1.assertDone();
5756
listener2.assertDone();

‎components/native-loader/src/test/java/datadog/nativeloader/NativeLoaderTest.java‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -320,11 +320,19 @@ public void fromJarBackedClassLoader() throws IOException, LibraryLoadException
320320
Path jar = jar("test-data");
321321
try {
322322
try (URLClassLoader classLoader = createClassLoader(jar)) {
323-
NativeLoader loader = NativeLoader.builder().fromClassLoader(classLoader).build();
324-
try (LibFile lib = loader.resolveDynamic("dummy")) {
323+
NativeLoader loader = NativeLoader.builder().fromClassLoader(classLoader).build();
324+
325+
TestLibraryLoadingListener scopedListener = new TestLibraryLoadingListener().
326+
expectResolveDynamic("dummy").
327+
expectTempFileCreated("dummy").
328+
expectTempFileCleanup("dummy");
329+
330+
try (LibFile lib = loader.resolveDynamic("dummy", scopedListener)) {
325331
// loaded from a jar, so copied to temp file
326332
assertTempFile(lib);
327333
}
334+
335+
scopedListener.assertDone();
328336
}
329337
} finally {
330338
deleteHelper(jar);

‎components/native-loader/src/test/java/datadog/nativeloader/TestLibraryLoadingListener.java‎

Lines changed: 48 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import static org.junit.jupiter.api.Assertions.assertEquals;
44
import static org.junit.jupiter.api.Assertions.assertNull;
55
import static org.junit.jupiter.api.Assertions.assertTrue;
6+
import static org.junit.jupiter.api.Assertions.fail;
67

78
import java.net.URL;
89
import java.nio.file.Path;
@@ -11,6 +12,9 @@
1112

1213
public final class TestLibraryLoadingListener implements LibraryLoadingListener {
1314
private final LinkedList<Check> checks;
15+
16+
private Check failedCheck = null;
17+
private Throwable failedCause = null;
1418

1519
TestLibraryLoadingListener() {
1620
this.checks = new LinkedList<>();
@@ -190,6 +194,16 @@ public TestLibraryLoadingListener copy() {
190194
}
191195

192196
public void assertDone() {
197+
if ( this.failedCheck != null ) {
198+
try {
199+
fail("check failed: " + this.failedCheck, this.failedCause);
200+
} catch ( AssertionError e ) {
201+
e.initCause(this.failedCause);
202+
203+
throw e;
204+
}
205+
}
206+
193207
// written this way for better debugging
194208
assertEquals(Collections.emptyList(), this.checks);
195209
}
@@ -201,8 +215,7 @@ public void onResolveDynamic(
201215
String libName,
202216
boolean isPreloaded,
203217
URL optionalUrl) {
204-
this.nextCheck()
205-
.onResolveDynamic(platformSpec, optionalComponent, libName, isPreloaded, optionalUrl);
218+
this.nextCheck(check -> check.onResolveDynamic(platformSpec, optionalComponent, libName, isPreloaded, optionalUrl));
206219
}
207220

208221
@Override
@@ -211,8 +224,7 @@ public void onResolveDynamicFailure(
211224
String optionalComponent,
212225
String libName,
213226
LibraryLoadException optionalCause) {
214-
this.nextCheck()
215-
.onResolveDynamicFailure(platformSpec, optionalComponent, libName, optionalCause);
227+
this.nextCheck(check -> check.onResolveDynamicFailure(platformSpec, optionalComponent, libName, optionalCause));
216228
}
217229

218230
@Override
@@ -222,7 +234,7 @@ public void onLoad(
222234
String libName,
223235
boolean isPreloaded,
224236
Path optionalLibPath) {
225-
this.nextCheck().onLoad(platformSpec, optionalComponent, libName, isPreloaded, optionalLibPath);
237+
this.nextCheck(check -> check.onLoad(platformSpec, optionalComponent, libName, isPreloaded, optionalLibPath));
226238
}
227239

228240
@Override
@@ -231,13 +243,15 @@ public void onLoadFailure(
231243
String optionalComponent,
232244
String libName,
233245
LibraryLoadException optionalCause) {
234-
this.nextCheck().onLoadFailure(platformSpec, optionalComponent, libName, optionalCause);
246+
this.nextCheck(check -> check.onLoadFailure(platformSpec, optionalComponent, libName, optionalCause));
235247
}
236248

237249
@Override
238250
public void onTempFileCreated(
239251
PlatformSpec platformSpec, String optionalComponent, String libName, Path tempFile) {
240-
this.nextCheck().onTempFileCreated(platformSpec, optionalComponent, libName, tempFile);
252+
if ( true ) new RuntimeException("onTempFileCreated!");
253+
254+
this.nextCheck(check -> check.onTempFileCreated(platformSpec, optionalComponent, libName, tempFile));
241255
}
242256

243257
@Override
@@ -249,30 +263,37 @@ public void onTempFileCreationFailure(
249263
String libExt,
250264
Path optionalTempFile,
251265
Throwable optionalCause) {
252-
this.nextCheck()
253-
.onTempFileCreationFailure(
266+
this.nextCheck(check -> check.onTempFileCreationFailure(
254267
platformSpec,
255268
optionalComponent,
256269
libName,
257270
tempDir,
258271
libExt,
259272
optionalTempFile,
260-
optionalCause);
273+
optionalCause));
261274
}
262275

263276
@Override
264277
public void onTempFileCleanup(
265278
PlatformSpec platformSpec, String optionalComponent, String libName, Path tempFile) {
266-
this.nextCheck().onTempFileCleanup(platformSpec, optionalComponent, libName, tempFile);
279+
this.nextCheck(check -> check.onTempFileCleanup(platformSpec, optionalComponent, libName, tempFile));
267280
}
268281

269282
TestLibraryLoadingListener addCheck(Check check) {
270283
this.checks.addLast(check);
271284
return this;
272285
}
273286

274-
Check nextCheck() {
275-
return this.checks.isEmpty() ? Check.NOTHING : this.checks.removeFirst();
287+
void nextCheck(CheckInvocation invocation) {
288+
Check nextCheck = this.checks.isEmpty() ? Check.NOTHING : this.checks.removeFirst();
289+
try {
290+
invocation.invoke(nextCheck);
291+
} catch ( Throwable t ) {
292+
if ( this.failedCheck == null ) {
293+
this.failedCheck = nextCheck;
294+
this.failedCause = t;
295+
}
296+
}
276297
}
277298

278299
static final class LibCheck {
@@ -319,6 +340,11 @@ public String toString() {
319340
}
320341
}
321342
}
343+
344+
@FunctionalInterface
345+
interface CheckInvocation {
346+
void invoke(Check check);
347+
}
322348

323349
abstract static class Check implements LibraryLoadingListener {
324350
static final Check NOTHING = new Check("nothing") {};
@@ -340,7 +366,7 @@ public void onLoad(
340366
String libName,
341367
boolean isPreloaded,
342368
Path optionalLibPath) {
343-
this.fail("onLoad", platformSpec, optionalComponent, libName, isPreloaded, optionalLibPath);
369+
this.fallback("onLoad", platformSpec, optionalComponent, libName, isPreloaded, optionalLibPath);
344370
}
345371

346372
@Override
@@ -349,7 +375,7 @@ public void onLoadFailure(
349375
String optionalComponent,
350376
String libName,
351377
LibraryLoadException optionalCause) {
352-
this.fail("onLoadFailure", platformSpec, optionalComponent, libName, optionalCause);
378+
this.fallback("onLoadFailure", platformSpec, optionalComponent, libName, optionalCause);
353379
}
354380

355381
@Override
@@ -359,7 +385,7 @@ public void onResolveDynamic(
359385
String libName,
360386
boolean isPreloaded,
361387
URL optionalUrl) {
362-
this.fail(
388+
this.fallback(
363389
"onResolveDynamic", platformSpec, optionalComponent, libName, isPreloaded, optionalUrl);
364390
}
365391

@@ -369,13 +395,13 @@ public void onResolveDynamicFailure(
369395
String optionalComponent,
370396
String libName,
371397
LibraryLoadException optionalCause) {
372-
this.fail("onResolveDynamicFailure", platformSpec, optionalComponent, libName, optionalCause);
398+
this.fallback("onResolveDynamicFailure", platformSpec, optionalComponent, libName, optionalCause);
373399
}
374400

375401
@Override
376402
public void onTempFileCreated(
377403
PlatformSpec platformSpec, String optionalComponent, String libName, Path tempFile) {
378-
this.fail("onTmepFileCreated", platformSpec, optionalComponent, libName, tempFile);
404+
this.fallback("onTmepFileCreated", platformSpec, optionalComponent, libName, tempFile);
379405
}
380406

381407
@Override
@@ -387,7 +413,7 @@ public void onTempFileCreationFailure(
387413
String libExt,
388414
Path optionalTempFile,
389415
Throwable optionalCause) {
390-
this.fail(
416+
this.fallback(
391417
"onTempFileCreationFailure",
392418
platformSpec,
393419
optionalComponent,
@@ -401,10 +427,10 @@ public void onTempFileCreationFailure(
401427
@Override
402428
public void onTempFileCleanup(
403429
PlatformSpec platformSpec, String optionalComponent, String libName, Path tempFile) {
404-
this.fail("onTempFileCleanup", platformSpec, optionalComponent, libName, tempFile);
430+
this.fallback("onTempFileCleanup", platformSpec, optionalComponent, libName, tempFile);
405431
}
406432

407-
void fail(String methodName, Object... args) {
433+
void fallback(String methodName, Object... args) {
408434
fail("unxpected call: " + callToString(methodName, args) + " - expected: " + this.name);
409435
}
410436

@@ -414,7 +440,7 @@ static final String callToString(String methodName, Object... args) {
414440
builder.append('(');
415441
for (int i = 0; i < args.length; ++i) {
416442
if (i != 0) builder.append(", ");
417-
builder.append(args[i].toString());
443+
builder.append(String.valueOf(args[i]));
418444
}
419445
builder.append(')');
420446
return builder.toString();

0 commit comments

Comments
 (0)