TFW: refactor for clarity

testify-iiop/src/main/java/testify/iiop/annotation/ServerComms.java

@@ -26,7 +26,7 @@
 import org.omg.CosNaming.NamingContextHelper;
 import org.omg.PortableServer.Servant;
 import org.opentest4j.TestAbortedException;
-import testify.annotation.logging.TestLogger;
+import testify.annotation.logging.LoggingController;
 import testify.bus.Bus;
 import testify.bus.EnumSpec;
 import testify.bus.FieldSpec;
@@ -90,8 +90,8 @@ private enum ServerRequest implements EnumSpec<ServerOp> {SEND}
     private enum MethodRequest implements MethodSpec {SEND}
     private enum FieldRequest implements FieldSpec {INIT}
     private enum ExportRequest implements MemberSpec {EXPORT}
-    private enum BeginLogging implements TypeSpec<Supplier<Optional<TestLogger>>> {BEGIN_LOGGING}
-    private enum EndLogging implements TypeSpec<Consumer<TestLogger>> {END_LOGGING}
+    private enum BeginLogging implements TypeSpec<Supplier<Optional<LoggingController>>> {BEGIN_LOGGING}
+    private enum EndLogging implements TypeSpec<Consumer<LoggingController>> {END_LOGGING}
     private enum Result implements TypeSpec<ServerSideException> {RESULT}
 
     private static final String REQUEST_COUNT_PREFIX = "Request#";
@@ -113,7 +113,7 @@ private enum Result implements TypeSpec<ServerSideException> {RESULT}
 
     private transient CountDownLatch serverShutdown;
     /** The logger to be used, per test. Server-side only! */
-    private transient Optional<TestLogger> testLogger;
+    private transient Optional<LoggingController> testLogger;
 
     private transient ServerComms otherSide; // can only be populated if the server is in the same process as the client
 
@@ -260,24 +260,24 @@ private Object invoke0(Method m) {
         return ReflectionSupport.invokeMethod(m, null, params);
     }
 
-    void beginLogging(Supplier<Optional<TestLogger>> supplier) {
+    void beginLogging(Supplier<Optional<LoggingController>> supplier) {
         assertClientSide();
         bus.put(BeginLogging.BEGIN_LOGGING, supplier);
         waitForCompletion(t -> new LoggingFailed("Failure beginning logging", t));
     }
 
-    void beginLogging0(Supplier<Optional<TestLogger>> supplier) {
+    void beginLogging0(Supplier<Optional<LoggingController>> supplier) {
         assertServer(IS_STARTED);
         this.testLogger = supplier.get();
     }
 
-    void endLogging(Consumer<TestLogger> consumer) {
+    void endLogging(Consumer<LoggingController> consumer) {
         assertClientSide();
         bus.put(EndLogging.END_LOGGING, consumer);
         waitForCompletion(t -> new LoggingFailed("Failure ending logging", t));
     }
 
-    void endLogging0(Consumer<TestLogger> consumer) {
+    void endLogging0(Consumer<LoggingController> consumer) {
         assertServer(IS_STARTED);
         this.testLogger.ifPresent(consumer);
         this.testLogger = null; // DELIBERATE! Invoking end without begin is an error

testify/src/main/java/testify/annotation/TestLogging.java → testify/src/main/java/testify/annotation/TraceTestify.java

@@ -29,7 +29,7 @@
 
 @Target({ANNOTATION_TYPE, TYPE})
 @Retention(RUNTIME)
-public @interface TestLogging {
+public @interface TraceTestify {
     /** A regular expression to match the classes to trace */
     String value() default ".*";
     TestLogLevel level() default DEFAULT;

testify/src/main/java/testify/annotation/impl/TestLoggingSteward.java

@@ -18,17 +18,17 @@
 package testify.annotation.impl;
 
 import org.junit.platform.commons.support.AnnotationSupport;
-import testify.annotation.TestLogging;
+import testify.annotation.TraceTestify;
 import testify.parts.PartRunner;
 
 import java.lang.reflect.AnnotatedElement;
 
 public enum TestLoggingSteward {
     ;
     public static void addTestLogSettings(PartRunner runner, AnnotatedElement elem) {
-        AnnotationSupport.findAnnotation(elem, TestLogging.class).ifPresent(trc -> TestLoggingSteward.addTestLogSettings(runner, trc));
+        AnnotationSupport.findAnnotation(elem, TraceTestify.class).ifPresent(trc -> TestLoggingSteward.addTestLogSettings(runner, trc));
     }
-    private static void addTestLogSettings(PartRunner runner, TestLogging config) {
+    private static void addTestLogSettings(PartRunner runner, TraceTestify config) {
         if (config.value().isEmpty()) return;
         runner.enableTestLogging(config.level(), config.value());
     }

testify/src/main/java/testify/annotation/logging/TestLogger.java → testify/src/main/java/testify/annotation/logging/LoggingController.java

@@ -17,8 +17,6 @@
  */
 package testify.annotation.logging;
 
-import org.junit.jupiter.api.extension.ExtensionContext;
-
 import java.io.PrintWriter;
 import java.util.ArrayDeque;
 import java.util.Deque;
@@ -29,9 +27,7 @@
 import java.util.logging.Level;
 import java.util.logging.LogRecord;
 import java.util.logging.Logger;
-import java.util.stream.Collectors;
 
-import static org.junit.platform.commons.support.AnnotationSupport.findRepeatableAnnotations;
 import static testify.util.Queues.drain;
 import static testify.util.Queues.drainInOrder;
 
@@ -40,7 +36,7 @@
  * as required by the annotation for that test.
  */
 @Logging
-public class TestLogger {
+class LoggingController {
     private final Handler handler = new Handler();
     private final PrintWriter out = new PrintWriter(System.out);
     private final Deque<List<LogSetting>> settingsStack = new ArrayDeque<>();
@@ -60,26 +56,12 @@ public class TestLogger {
 
     void registerLogHandler() { Logger.getLogger("").addHandler(handler); }
     void deregisterLogHandler() { Logger.getLogger("").removeHandler(handler); }
+    void pushSettings(List<LogSetting> settings) { settingsStack.push(settings);}
+    void popSettings() { settingsStack.pop().forEach(LogSetting::undo);}
 
-    public void before(ExtensionContext ctx) {
-        List<LogSetting> settings = findRepeatableAnnotations(ctx.getElement(), Logging.class)
-                        .stream()
-                        .map(LogSetting::new)
-                        .collect(Collectors.toList());
-        settingsStack.push(settings);
-        flushLogs("BEFORE: " + ctx.getDisplayName());
-    }
-
-    public void after(ExtensionContext ctx) {
-        flushLogs("AFTER: " + ctx.getDisplayName());
-        settingsStack.pop().forEach(LogSetting::undo);
-    }
-
-    public void somethingWentWrong(Throwable throwable) {
-        this.badStuffHappened = true;
-    }
+    void somethingWentWrong(Throwable throwable) { this.badStuffHappened = true; }
 
-    private void flushLogs(String displayName) {
+    void flushLogs(String displayName) {
         // if there were no log settings, do nothing at all
         if (settingsStack.stream().allMatch(List::isEmpty)) return;
 
@@ -91,7 +73,7 @@ private void flushLogs(String displayName) {
             return;
         }
 
-        char flag = badStuffHappened ? '\u274C' : '\u2714';
+        char flag = badStuffHappened ? '\u274C' : '\u2714'; // cross or tick character
         badStuffHappened = false;
         // PRINT THREAD KEY
         drain(newThreads).forEach(this::introduceThread);
@@ -129,7 +111,7 @@ public void flush() {}
         public void close() throws SecurityException {}
     }
 
-    private class LogSetting {
+    static class LogSetting {
         private final Logger logger;
         private final Level oldLevel;
 

testify/src/main/java/testify/annotation/logging/LoggingExtension.java

@@ -26,20 +26,39 @@
 import org.junit.jupiter.api.extension.ExtensionContext.Store.CloseableResource;
 import org.junit.jupiter.api.extension.TestExecutionExceptionHandler;
 
-public final class LoggingExtension extends TestLogger implements CloseableResource, BeforeAllCallback, BeforeTestExecutionCallback, AfterTestExecutionCallback, AfterAllCallback, TestExecutionExceptionHandler {
+import java.util.List;
+import java.util.stream.Collectors;
+
+import static org.junit.platform.commons.support.AnnotationSupport.findRepeatableAnnotations;
+
+public final class LoggingExtension implements CloseableResource, BeforeAllCallback, BeforeTestExecutionCallback, AfterTestExecutionCallback, AfterAllCallback, TestExecutionExceptionHandler {
+    private final LoggingController controller = new LoggingController();
+
     public void beforeAll(ExtensionContext ctx) {
-        registerLogHandler();
-        this.before(ctx);
-        ctx.getStore(Namespace.create(this)).put(this, this); // ensure close() gets called
+        controller.registerLogHandler();
+        startLogging(ctx);
+        ctx.getStore(Namespace.create(this)).put(this, this); // the namespace will call close() during cleanup
     }
-    public void beforeTestExecution(ExtensionContext ctx) { this.before(ctx); }
-    public void afterTestExecution(ExtensionContext ctx) { this.after(ctx); }
-    public void afterAll(ExtensionContext ctx) { this.after(ctx); }
+    public void beforeTestExecution(ExtensionContext ctx) { startLogging(ctx); }
+    public void afterTestExecution(ExtensionContext ctx) { endLogging(ctx); }
+    public void afterAll(ExtensionContext ctx) { endLogging(ctx); }
     public void handleTestExecutionException(ExtensionContext ctx, Throwable throwable) throws Throwable {
-        this.somethingWentWrong(throwable);
+        controller.somethingWentWrong(throwable);
         throw throwable; // rethrow or tests won't fail
     }
-    public void close() throws Throwable {
-        deregisterLogHandler();
+    public void close() throws Throwable { controller.deregisterLogHandler(); }
+
+    private void startLogging(ExtensionContext ctx) {
+        List<LoggingController.LogSetting> settings = findRepeatableAnnotations(ctx.getElement(), Logging.class)
+                        .stream()
+                        .map(LoggingController.LogSetting::new)
+                        .collect(Collectors.toList());
+        controller.pushSettings(settings);
+        controller.flushLogs("BEFORE: " + ctx.getDisplayName());
+    }
+
+    private void endLogging(ExtensionContext ctx) {
+        controller.flushLogs("AFTER: " + ctx.getDisplayName());
+        controller.popSettings();
     }
 }

testify/src/main/java/testify/parts/ForkedPart.java

@@ -25,5 +25,13 @@
  * Interface to customise the behaviour of a {@link PartRunner} when forking a {@link TestPart}.
  */
 public interface ForkedPart {
+
+    // TODO: add support for JUnit hook points here - at least @Before and @After
+
+
+    /**
+     * Run the provided action to shut down this part.
+     * It will be run locally, by on the thread that calls {@link PartRunner#join()}.
+     */
     void endWith(Consumer<Bus> endAction);
 }

testify/src/main/java/testify/parts/PartRunner.java

@@ -21,7 +21,12 @@
 import testify.bus.TestLogLevel;
 
 import java.lang.reflect.InvocationTargetException;
+import java.util.function.Consumer;
 
+/**
+ * An object that can run a {@link TestPart} in another context.
+ * Methods are provided for configuring what that contet should be.
+ */
 @SuppressWarnings("UnusedReturnValue")
 public interface PartRunner {
     static PartRunner create() { return new PartRunnerImpl(); }
@@ -59,5 +64,9 @@ static TestPart wrapMain(Class<?> mainClass, String[] args) {
         };
     }
 
+    /**
+     * Calls any actions registered using {@link ForkedPart#endWith(Consumer)}.
+     * Wait for all the running parts to complete.
+     */
     void join();
 }

testify/src/main/java/testify/parts/ProcessRunner.java

@@ -39,6 +39,12 @@ public class ProcessRunner implements Runner<Process>{
     private enum Part implements TypeSpec<NamedPart> {NAMED_PART}
     private static final List<String> PROPERTIES_TO_COPY = Collections.singletonList("java.endorsed.dirs");
 
+    /**
+     * This is run on the remote process.
+     * @param args
+     *             The first argument is the name of the process.
+     *             Other arguments are ignored.
+     */
     public static void main(String[] args) {
         String name = args[0];
         Bus bus = InterProcessBus.createSlave().forUser(name);

testify/src/main/java/testify/parts/Runner.java

@@ -21,6 +21,17 @@
 
 import java.util.concurrent.TimeUnit;
 
+/**
+ * The main idea of the Testify framework — allow a piece of work to be executed in another context, such as:
+ * <ul>
+ *     <li>another thread</li>
+ *     <li>another process</li>
+ * </ul>
+ *
+ * This interface abstracts that idea to allow that work to be started, awaited, or terminated.
+ *
+ * @param <J> the runtime type representing the context, such as a {@link Thread} or {@link Process}
+ */
 interface Runner<J> {
     J fork(InterProcessBus centralBus, NamedPart part);
 

testify/src/main/java/testify/streams/Streams.java

@@ -39,9 +39,9 @@ private interface Streamer0<T> extends Spliterator<T> {
     }
 
     public static <T> Stream<T> stream(Streamer<T> streamer) {
-        // N.B. pay special attention to the cast below.
-        // It is casting a *method reference* to a functional interface,
-        // which is Java 8's way of adding a mixin.
-        return StreamSupport.stream((Streamer0<T>) streamer::tryAdvance, false);
+        // N.B. implicitly casting a method reference (or lambda) to a functional interface is a way of adding a mixin.
+        // The calling code cannot override the default methods on the private interface, Streamer0.
+        Streamer0<T> spliterator = streamer::tryAdvance;
+        return StreamSupport.stream(spliterator, false);
     }
 }

yoko-core/src/test/java-testify/org/apache/yoko/FullValueDescriptorTest.java

@@ -37,7 +37,7 @@
 import org.omg.PortableServer.POAHelper;
 import org.omg.PortableServer.Servant;
 import testify.bus.Bus;
-import testify.annotation.TestLogging;
+import testify.annotation.TraceTestify;
 import testify.iiop.annotation.ConfigureOrb;
 import testify.iiop.annotation.ConfigureServer;
 import testify.iiop.annotation.ConfigureServer.BeforeServer;
@@ -54,7 +54,7 @@
 import static testify.annotation.logging.Logging.LoggingLevel.FINEST;
 
 @ConfigureServer(serverOrb = @ConfigureOrb(nameService = READ_WRITE))
-@TestLogging
+@TraceTestify
 @Logging(value = "yoko.verbose.data", level = FINEST)
 @TestInstance(Lifecycle.PER_CLASS) // this allows @BeforeAll on an instance method
 public class FullValueDescriptorTest {

yoko-core/src/test/java-testify/org/apache/yoko/TypesTest.java

@@ -24,11 +24,11 @@
 import test.types.TestTypeCode;
 import test.types.TestUnion;
 import testify.annotation.ConfigurePartRunner;
-import testify.annotation.TestLogging;
+import testify.annotation.TraceTestify;
 import testify.parts.PartRunner;
 
 @ConfigurePartRunner
-@TestLogging
+@TraceTestify
 class TypesTest {
     @Test
     void testTypeCode(PartRunner runner) { runner.forkMain(TestTypeCode.class); }

yoko-core/src/test/java-testify/org/apache/yoko/orb/OCI/IIOP/FragmentedMessageTest.java

@@ -31,7 +31,7 @@
 import testify.bus.Bus;
 import testify.bus.StringSpec;
 import testify.iiop.TestIORInterceptor;
-import testify.annotation.TestLogging;
+import testify.annotation.TraceTestify;
 import testify.iiop.annotation.ConfigureOrb.UseWithOrb;
 import testify.iiop.annotation.ConfigureServer;
 import testify.iiop.annotation.ConfigureServer.RemoteImpl;
@@ -53,7 +53,7 @@
  * fragments before sending them on.
  */
 @ConfigureServer
-@TestLogging
+@TraceTestify
 public class FragmentedMessageTest {
     interface Echo extends RemoteFunction<String, String> {}