From e08f19570f8433506bfd53434d36398e1f6248d1 Mon Sep 17 00:00:00 2001 From: Luis <1105281+lpenap@users.noreply.github.com> Date: Wed, 16 Sep 2026 15:24:14 -0300 Subject: [PATCH] Make the remaining excluded classes testable and cover them Consumer now stops when its thread is interrupted instead of looping forever and swallowing the interruption, and ProducerConsumerExampleRunner waits for production and consumption to finish, then shuts the executor down so the pool threads no longer leak on every run. HyperlinkMouseListener gets a package-private LinkOpener seam with Desktop.browse as the default, so the click handler can be tested without a desktop. The error log now includes the exception instead of the mouse event. New tests cover ExamplesCommandLineRunner through a GenericApplicationContext with a fake runner bean and all logback level branches, Consumer, ProducerConsumerExampleRunner including thread cleanup, the click paths of HyperlinkMouseListener and ObservableAbstract.removeAllListeners. The corresponding JaCoCo excludes are removed; only the GUI shell remains excluded. Co-Authored-By: Claude Fable 5.1 --- pom.xml | 5 - .../app/ui/HyperlinkMouseListener.java | 28 +++-- .../constructs/producerconsumer/Consumer.java | 13 ++- .../ProducerConsumerExampleRunner.java | 32 +++--- .../app/ExamplesCommandLineRunnerTests.java | 103 ++++++++++++++++++ .../app/ui/HyperlinkMouseListenerTests.java | 51 +++++++++ .../constructs/observer/ObserverTests.java | 12 ++ .../producerconsumer/ConsumerTests.java | 50 +++++++++ .../ProducerConsumerExampleRunnerTests.java | 31 ++++++ 9 files changed, 294 insertions(+), 31 deletions(-) create mode 100644 src/test/java/com/penapereira/example/constructs/app/ExamplesCommandLineRunnerTests.java create mode 100644 src/test/java/com/penapereira/example/constructs/producerconsumer/ConsumerTests.java create mode 100644 src/test/java/com/penapereira/example/constructs/producerconsumer/ProducerConsumerExampleRunnerTests.java diff --git a/pom.xml b/pom.xml index 69688d2..e0f2201 100644 --- a/pom.xml +++ b/pom.xml @@ -109,13 +109,8 @@ **/AppCommandLineRunner* - **/ExamplesCommandLineRunner* - **/ProducerConsumerExampleRunner* - **/Consumer* **/JavaPatternsAndConstructsApplication* **/MainWindow* - **/HyperlinkMouseListener* - **/ObservableAbstract* diff --git a/src/main/java/com/penapereira/example/constructs/app/ui/HyperlinkMouseListener.java b/src/main/java/com/penapereira/example/constructs/app/ui/HyperlinkMouseListener.java index 4c2bcdc..fc93566 100644 --- a/src/main/java/com/penapereira/example/constructs/app/ui/HyperlinkMouseListener.java +++ b/src/main/java/com/penapereira/example/constructs/app/ui/HyperlinkMouseListener.java @@ -17,23 +17,34 @@ public class HyperlinkMouseListener implements MouseListener { - private Logger log = LoggerFactory.getLogger(HyperlinkMouseListener.class); + /** Opens a link in the user's browser. Replaceable in tests, where there is no desktop. */ + @FunctionalInterface + interface LinkOpener { + void open(URI uri) throws IOException; + } - ApplicationProperties props; + private static final Logger log = LoggerFactory.getLogger(HyperlinkMouseListener.class); + private final ApplicationProperties props; + private final LinkOpener linkOpener; private String lastText; public HyperlinkMouseListener(ApplicationProperties props) { + this(props, uri -> Desktop.getDesktop().browse(uri)); + } + + HyperlinkMouseListener(ApplicationProperties props, LinkOpener linkOpener) { this.props = props; + this.linkOpener = linkOpener; } @Override public void mouseClicked(MouseEvent e) { log.debug("Hyperlink text: " + lastText); try { - Desktop.getDesktop().browse(new URI(lastText)); - } catch (IOException | URISyntaxException e1) { - log.error("Error opening link", e); + linkOpener.open(new URI(lastText)); + } catch (IOException | URISyntaxException | RuntimeException ex) { + log.error("Error opening link", ex); } } @@ -53,11 +64,12 @@ public void mouseExited(MouseEvent e) { } @Override - public void mousePressed(MouseEvent arg0) { + public void mousePressed(MouseEvent e) { + // Nothing to do: the link opens on click. } @Override - public void mouseReleased(MouseEvent arg0) { + public void mouseReleased(MouseEvent e) { + // Nothing to do: the link opens on click. } - } diff --git a/src/main/java/com/penapereira/example/constructs/producerconsumer/Consumer.java b/src/main/java/com/penapereira/example/constructs/producerconsumer/Consumer.java index 78e9c91..4e110d0 100644 --- a/src/main/java/com/penapereira/example/constructs/producerconsumer/Consumer.java +++ b/src/main/java/com/penapereira/example/constructs/producerconsumer/Consumer.java @@ -9,24 +9,27 @@ public class Consumer implements Runnable { private static final Logger log = LoggerFactory.getLogger(Consumer.class); - private BlockingQueue queue; - - private int myId; + private final BlockingQueue queue; + private final int myId; public Consumer(BlockingQueue blockingQueue, int myId) { this.queue = blockingQueue; this.myId = myId; } + /** + * Consumes integers until the thread is interrupted, which is how the + * example runner asks the consumers to stop once the producer is done. + */ @Override public void run() { - while (true) { + while (!Thread.currentThread().isInterrupted()) { try { var consumed = queue.take(); log.trace(String.format(" %d: Consumed [%2d]", myId, consumed)); } catch (InterruptedException finish) { + Thread.currentThread().interrupt(); } } } - } diff --git a/src/main/java/com/penapereira/example/constructs/producerconsumer/ProducerConsumerExampleRunner.java b/src/main/java/com/penapereira/example/constructs/producerconsumer/ProducerConsumerExampleRunner.java index 72e2185..76f6c3f 100644 --- a/src/main/java/com/penapereira/example/constructs/producerconsumer/ProducerConsumerExampleRunner.java +++ b/src/main/java/com/penapereira/example/constructs/producerconsumer/ProducerConsumerExampleRunner.java @@ -18,20 +18,26 @@ public class ProducerConsumerExampleRunner implements ExampleRunnerInterface { private static final Logger log = LoggerFactory.getLogger(ProducerConsumerExampleRunner.class); @Override - public void runExample() throws InterruptedException { + public void runExample() throws Exception { log.trace("Executing Producer/Consumer implementation:"); - - BlockingQueue blockingQueue = new LinkedBlockingDeque(3); + BlockingQueue blockingQueue = new LinkedBlockingDeque<>(3); ExecutorService executor = Executors.newFixedThreadPool(3); - - Consumer consumer1 = new Consumer(blockingQueue, 1); - Consumer consumer2 = new Consumer(blockingQueue, 2); - Producer producer = new Producer(blockingQueue); - - executor.execute(consumer1); - executor.execute(consumer2); - executor.execute(producer); - - executor.awaitTermination(1, TimeUnit.SECONDS); + try { + executor.execute(new Consumer(blockingQueue, 1)); + executor.execute(new Consumer(blockingQueue, 2)); + // Wait until the producer has put every item in the queue... + executor.submit(new Producer(blockingQueue)).get(); + // ...and until the consumers have taken all of them. + while (!blockingQueue.isEmpty()) { + Thread.sleep(10); + } + } finally { + // Consumers block forever waiting for more work; interrupt them so + // the pool threads do not leak after the example finishes. + executor.shutdownNow(); + if (!executor.awaitTermination(1, TimeUnit.SECONDS)) { + log.warn("Consumers did not stop in time"); + } + } } } diff --git a/src/test/java/com/penapereira/example/constructs/app/ExamplesCommandLineRunnerTests.java b/src/test/java/com/penapereira/example/constructs/app/ExamplesCommandLineRunnerTests.java new file mode 100644 index 0000000..ed71d4d --- /dev/null +++ b/src/test/java/com/penapereira/example/constructs/app/ExamplesCommandLineRunnerTests.java @@ -0,0 +1,103 @@ +package com.penapereira.example.constructs.app; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.concurrent.atomic.AtomicInteger; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.slf4j.LoggerFactory; +import org.springframework.context.support.GenericApplicationContext; + +import com.penapereira.example.constructs.app.properties.ApplicationProperties; +import com.penapereira.example.constructs.app.properties.Messages; + +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; + +class ExamplesCommandLineRunnerTests { + + private final Logger runnerLog = (Logger) LoggerFactory.getLogger(ExamplesCommandLineRunner.class); + private final Level originalLevel = runnerLog.getLevel(); + private final ListAppender logged = new ListAppender<>(); + + private final AtomicInteger executions = new AtomicInteger(); + private GenericApplicationContext ctx; + private ApplicationProperties props; + private Messages msg; + + @BeforeEach + void setUp() { + logged.start(); + runnerLog.addAppender(logged); + + ctx = new GenericApplicationContext(); + ctx.registerBean("fakeExampleRunner", ExampleRunnerInterface.class, () -> executions::incrementAndGet); + ctx.refresh(); + + props = new ApplicationProperties(); + msg = new Messages(); + msg.setExamplesFound("Examples"); + msg.setSeparator("---"); + msg.setEnableTraceToSeeExamplesDetails("enable trace"); + msg.setEnableDebugToSeeExamplesList("enable debug"); + } + + @AfterEach + void tearDown() { + runnerLog.detachAppender(logged); + runnerLog.setLevel(originalLevel); + ctx.close(); + } + + private ExamplesCommandLineRunner runner() { + return new ExamplesCommandLineRunner(ctx, msg, props); + } + + private boolean logged(String text) { + return logged.list.stream().anyMatch(e -> e.getFormattedMessage().contains(text)); + } + + @Test + void doesNothingWhenDisabled() throws Exception { + props.setEnableCommandLineRunner(false); + + runner().run(); + + assertEquals(0, executions.get()); + assertTrue(logged.list.isEmpty()); + } + + @Test + void listsAndExecutesExamplesWhenTraceIsEnabled() throws Exception { + props.setEnableCommandLineRunner(true); + runnerLog.setLevel(Level.TRACE); + + runner().run(); + + assertEquals(1, executions.get()); + assertTrue(logged("Examples (ExampleRunnerInterface):")); + assertTrue(logged(" 1 : fake")); + assertTrue(logged("---")); + assertFalse(logged("enable trace")); + assertFalse(logged("enable debug")); + } + + @Test + void tellsHowToSeeDetailsWhenLogLevelIsTooHigh() throws Exception { + props.setEnableCommandLineRunner(true); + runnerLog.setLevel(Level.INFO); + + runner().run(); + + assertEquals(1, executions.get()); + assertTrue(logged("enable debug")); + assertTrue(logged("enable trace")); + assertFalse(logged(" 1 : fake")); + } +} diff --git a/src/test/java/com/penapereira/example/constructs/app/ui/HyperlinkMouseListenerTests.java b/src/test/java/com/penapereira/example/constructs/app/ui/HyperlinkMouseListenerTests.java index 150228c..995bd9c 100644 --- a/src/test/java/com/penapereira/example/constructs/app/ui/HyperlinkMouseListenerTests.java +++ b/src/test/java/com/penapereira/example/constructs/app/ui/HyperlinkMouseListenerTests.java @@ -4,6 +4,9 @@ import java.awt.Color; import java.awt.event.MouseEvent; +import java.io.IOException; +import java.net.URI; +import java.util.concurrent.atomic.AtomicReference; import javax.swing.JLabel; @@ -30,4 +33,52 @@ void hyperlinkChangesColorAndText() { assertEquals("http://example.com", label.getText()); assertEquals(Color.decode(props.getLinkColor()), label.getForeground()); } + + private static ApplicationProperties props() { + ApplicationProperties props = new ApplicationProperties(); + props.setLinkColor("#000000"); + props.setLinkColorHover("#ffffff"); + return props; + } + + private static MouseEvent eventOn(JLabel label, int id) { + return new MouseEvent(label, id, 0, 0, 0, 0, 1, false); + } + + @Test + void clickOpensTheLinkShownWhenTheMouseEntered() { + AtomicReference opened = new AtomicReference<>(); + HyperlinkMouseListener listener = new HyperlinkMouseListener(props(), opened::set); + JLabel label = new JLabel("http://example.com"); + + listener.mouseEntered(eventOn(label, MouseEvent.MOUSE_ENTERED)); + listener.mousePressed(eventOn(label, MouseEvent.MOUSE_PRESSED)); + listener.mouseReleased(eventOn(label, MouseEvent.MOUSE_RELEASED)); + listener.mouseClicked(eventOn(label, MouseEvent.MOUSE_CLICKED)); + + assertEquals(URI.create("http://example.com"), opened.get()); + } + + @Test + void clickOnMalformedLinkIsLoggedNotThrown() { + AtomicReference opened = new AtomicReference<>(); + HyperlinkMouseListener listener = new HyperlinkMouseListener(props(), opened::set); + JLabel label = new JLabel("http://exa mple.com"); + + listener.mouseEntered(eventOn(label, MouseEvent.MOUSE_ENTERED)); + assertDoesNotThrow(() -> listener.mouseClicked(eventOn(label, MouseEvent.MOUSE_CLICKED))); + + assertNull(opened.get()); + } + + @Test + void failureToOpenTheBrowserIsLoggedNotThrown() { + HyperlinkMouseListener listener = new HyperlinkMouseListener(props(), uri -> { + throw new IOException("no browser"); + }); + JLabel label = new JLabel("http://example.com"); + + listener.mouseEntered(eventOn(label, MouseEvent.MOUSE_ENTERED)); + assertDoesNotThrow(() -> listener.mouseClicked(eventOn(label, MouseEvent.MOUSE_CLICKED))); + } } diff --git a/src/test/java/com/penapereira/example/constructs/observer/ObserverTests.java b/src/test/java/com/penapereira/example/constructs/observer/ObserverTests.java index b095c25..e8f6277 100644 --- a/src/test/java/com/penapereira/example/constructs/observer/ObserverTests.java +++ b/src/test/java/com/penapereira/example/constructs/observer/ObserverTests.java @@ -31,4 +31,16 @@ void listenerCanBeRemoved() { subject.doSomethingWith(1); // should not throw assertEquals(0, subject.getSupport().getPropertyChangeListeners().length); } + + @Test + void allListenersCanBeRemovedAtOnce() { + Observable subject = new Observable(); + subject.addPropertyChangeListener(new Observer()); + subject.addPropertyChangeListener(event -> {}); + assertEquals(2, subject.getSupport().getPropertyChangeListeners().length); + + subject.removeAllListeners(); + + assertEquals(0, subject.getSupport().getPropertyChangeListeners().length); + } } diff --git a/src/test/java/com/penapereira/example/constructs/producerconsumer/ConsumerTests.java b/src/test/java/com/penapereira/example/constructs/producerconsumer/ConsumerTests.java new file mode 100644 index 0000000..6f8a99c --- /dev/null +++ b/src/test/java/com/penapereira/example/constructs/producerconsumer/ConsumerTests.java @@ -0,0 +1,50 @@ +package com.penapereira.example.constructs.producerconsumer; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.concurrent.BlockingQueue; +import java.util.concurrent.LinkedBlockingDeque; + +import org.junit.jupiter.api.Test; + +class ConsumerTests { + + private static void waitUntilBlockedOnQueue(Thread t) { + while (t.getState() != Thread.State.WAITING) { + Thread.onSpinWait(); + } + } + + @Test + void consumesEverythingAndStopsWhenInterrupted() throws InterruptedException { + BlockingQueue queue = new LinkedBlockingDeque<>(); + queue.put(1); + queue.put(2); + Thread t = new Thread(new Consumer(queue, 1)); + + t.start(); + waitUntilBlockedOnQueue(t); + assertTrue(queue.isEmpty()); + + t.interrupt(); + t.join(5_000); + assertFalse(t.isAlive()); + } + + @Test + void stopsImmediatelyWhenInterruptedBeforeStarting() throws InterruptedException { + BlockingQueue queue = new LinkedBlockingDeque<>(); + queue.put(1); + Thread t = new Thread(() -> { + Thread.currentThread().interrupt(); + new Consumer(queue, 2).run(); + }); + + t.start(); + t.join(5_000); + + assertFalse(t.isAlive()); + assertFalse(queue.isEmpty()); + } +} diff --git a/src/test/java/com/penapereira/example/constructs/producerconsumer/ProducerConsumerExampleRunnerTests.java b/src/test/java/com/penapereira/example/constructs/producerconsumer/ProducerConsumerExampleRunnerTests.java new file mode 100644 index 0000000..4c3dcd3 --- /dev/null +++ b/src/test/java/com/penapereira/example/constructs/producerconsumer/ProducerConsumerExampleRunnerTests.java @@ -0,0 +1,31 @@ +package com.penapereira.example.constructs.producerconsumer; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively; + +import java.time.Duration; + +import org.junit.jupiter.api.Test; + +class ProducerConsumerExampleRunnerTests { + + private static long consumerThreads() { + return Thread.getAllStackTraces().keySet().stream() + .filter(t -> t.isAlive() && t.getName().startsWith("pool-")) + .count(); + } + + @Test + void runsToCompletionAndStopsItsThreads() { + long before = consumerThreads(); + + assertTimeoutPreemptively(Duration.ofSeconds(10), () -> new ProducerConsumerExampleRunner().runExample()); + + // Worker threads may still be exiting right after the pool reports termination. + long deadline = System.currentTimeMillis() + 5_000; + while (consumerThreads() != before && System.currentTimeMillis() < deadline) { + Thread.onSpinWait(); + } + assertEquals(before, consumerThreads()); + } +}