From c75dc223bc5a5bde215c6197bf35519bb7eb92ea Mon Sep 17 00:00:00 2001 From: Firas Regaieg Date: Fri, 13 Mar 2026 19:25:26 +0100 Subject: [PATCH 1/3] Adding a first unit test for Metrics with logging --- application/build.gradle | 1 + .../tjbot/features/analytics/Metrics.java | 3 + .../tjbot/features/MetricsTests.java | 118 ++++++++++++++++++ application/src/test/resources/log4j2.xml | 2 + 4 files changed, 124 insertions(+) create mode 100644 application/src/test/java/org/togetherjava/tjbot/features/MetricsTests.java diff --git a/application/build.gradle b/application/build.gradle index a387e78a87..47906193b0 100644 --- a/application/build.gradle +++ b/application/build.gradle @@ -80,6 +80,7 @@ dependencies { implementation 'org.apache.commons:commons-text:1.15.0' implementation 'com.apptasticsoftware:rssreader:3.12.0' + testImplementation 'org.assertj:assertj-core:3.27.7' testImplementation 'org.mockito:mockito-core:5.23.0' testImplementation "org.junit.jupiter:junit-jupiter-api:$junitVersion" testImplementation "org.junit.jupiter:junit-jupiter-params:$junitVersion" diff --git a/application/src/main/java/org/togetherjava/tjbot/features/analytics/Metrics.java b/application/src/main/java/org/togetherjava/tjbot/features/analytics/Metrics.java index 9aa0c797fe..d7d1f55cb0 100644 --- a/application/src/main/java/org/togetherjava/tjbot/features/analytics/Metrics.java +++ b/application/src/main/java/org/togetherjava/tjbot/features/analytics/Metrics.java @@ -53,4 +53,7 @@ private void processEvent(String event, Instant happenedAt) { .insert()); } + public ExecutorService getExecutorService() { + return service; + } } diff --git a/application/src/test/java/org/togetherjava/tjbot/features/MetricsTests.java b/application/src/test/java/org/togetherjava/tjbot/features/MetricsTests.java new file mode 100644 index 0000000000..78ae3dac0f --- /dev/null +++ b/application/src/test/java/org/togetherjava/tjbot/features/MetricsTests.java @@ -0,0 +1,118 @@ +package org.togetherjava.tjbot.features; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import org.togetherjava.tjbot.db.Database; +import org.togetherjava.tjbot.db.generated.tables.MetricEvents; +import org.togetherjava.tjbot.features.analytics.Metrics; + +import java.time.Duration; +import java.time.Instant; +import java.time.temporal.ChronoUnit; +import java.util.List; +import java.util.concurrent.locks.LockSupport; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.within; +import static org.junit.jupiter.api.Assertions.fail; + +final class MetricsTests { + private static final Logger logger = LoggerFactory.getLogger(MetricsTests.class); + + private static final Duration WAIT_TIMEOUT = Duration.ofSeconds(3); + + private Database database; + private Metrics metrics; + + @BeforeEach + void setUp() { + database = Database.createMemoryDatabase(MetricEvents.METRIC_EVENTS); + metrics = new Metrics(database); + } + + @AfterEach + void tearDown() { + metrics.getExecutorService().shutdownNow(); + } + + @Test + void countInsertsSingleEvent() { + + final String slashPing = "slash-ping"; + + metrics.count(slashPing); + + awaitRecords(1); + + List recordedEvents = readEventsOrderedById(); + + assertThat(recordedEvents).as("Metrics should persist the counted event in insertion order") + .containsExactly(slashPing); + + assertThat(readLatestEventHappenedAt()) + .as("Metrics should store a recent timestamp for event '%s' (recordedEvents=%s)", + slashPing, recordedEvents) + .isNotNull() + .isCloseTo(Instant.now(), within(5, ChronoUnit.SECONDS)); + } + + private void awaitRecords(int expectedAmount) { + Instant deadline = Instant.now().plus(WAIT_TIMEOUT); + + while (Instant.now().isBefore(deadline)) { + if (readRecordCount() == expectedAmount) { + return; + } + + LockSupport.parkNanos(Duration.ofMillis(25).toNanos()); + + if (Thread.interrupted()) { + int actualCount = readRecordCount(); + + String msg = String.format( + "Interrupted while waiting for metrics writes (expectedAmount=%d, actualCount=%d, timeout=%s, events=%s)", + expectedAmount, actualCount, WAIT_TIMEOUT, readEventsOrderedById()); + + logger.warn(msg); + + fail(msg); + } + } + + int actualCount = readRecordCount(); + + List recordedEvents = readEventsOrderedById(); + + String timeoutMessage = String.format( + "Timed out waiting for metrics writes (expectedAmount=%d, actualCount=%d, timeout=%s, events=%s)", + expectedAmount, actualCount, WAIT_TIMEOUT, recordedEvents); + + logger.warn(timeoutMessage); + + assertThat(actualCount).as(timeoutMessage).isEqualTo(expectedAmount); + } + + private int readRecordCount() { + return database.read(context -> context.fetchCount(MetricEvents.METRIC_EVENTS)); + } + + private List readEventsOrderedById() { + return database.read(context -> context.select(MetricEvents.METRIC_EVENTS.EVENT) + .from(MetricEvents.METRIC_EVENTS) + .orderBy(MetricEvents.METRIC_EVENTS.ID.asc()) + .fetch(MetricEvents.METRIC_EVENTS.EVENT)); + + } + + private Instant readLatestEventHappenedAt() { + return database.read(context -> context.select(MetricEvents.METRIC_EVENTS.HAPPENED_AT) + .from(MetricEvents.METRIC_EVENTS) + .orderBy(MetricEvents.METRIC_EVENTS.ID.desc()) + .limit(1) + .fetchOne(MetricEvents.METRIC_EVENTS.HAPPENED_AT)); + } +} diff --git a/application/src/test/resources/log4j2.xml b/application/src/test/resources/log4j2.xml index 586ab0cc45..9aa5025e0b 100644 --- a/application/src/test/resources/log4j2.xml +++ b/application/src/test/resources/log4j2.xml @@ -13,6 +13,8 @@ + + From 29bc42e3afae4e9fc4d1f42b0eefbb6b413e619c Mon Sep 17 00:00:00 2001 From: Firas Regaieg Date: Mon, 27 Apr 2026 21:15:20 +0100 Subject: [PATCH 2/3] Updates metrics test: using doAsync flag to check event persistence only; --- application/build.gradle | 1 - .../tjbot/features/analytics/Metrics.java | 48 +++++++--- .../tjbot/features/MetricsTests.java | 95 +++---------------- application/src/test/resources/log4j2.xml | 2 - 4 files changed, 49 insertions(+), 97 deletions(-) diff --git a/application/build.gradle b/application/build.gradle index 8fce3704fa..0770a382fb 100644 --- a/application/build.gradle +++ b/application/build.gradle @@ -80,7 +80,6 @@ dependencies { implementation 'org.apache.commons:commons-text:1.15.0' implementation 'com.apptasticsoftware:rssreader:3.12.0' - testImplementation 'org.assertj:assertj-core:3.27.7' testImplementation 'org.mockito:mockito-core:5.23.0' testImplementation "org.junit.jupiter:junit-jupiter-api:$junitVersion" testImplementation "org.junit.jupiter:junit-jupiter-params:$junitVersion" diff --git a/application/src/main/java/org/togetherjava/tjbot/features/analytics/Metrics.java b/application/src/main/java/org/togetherjava/tjbot/features/analytics/Metrics.java index c1ccf7483b..4d4e5a503c 100644 --- a/application/src/main/java/org/togetherjava/tjbot/features/analytics/Metrics.java +++ b/application/src/main/java/org/togetherjava/tjbot/features/analytics/Metrics.java @@ -44,6 +44,26 @@ public void count(String event) { count(event, Map.of()); } + /** + * Track an event execution with dimensions provided. + * + * @param event the event to save + * @param dimensions the dimensions to save + */ + public void count(String event, Map dimensions) { + count(event, dimensions, true); + } + + /** + * Track an event execution with flag for async execution. + * + * @param event the event to save + * @param doAsync the async flag + */ + public void count(String event, boolean doAsync) { + count(event, Map.of(), doAsync); + } + /** * Track an event execution with additional contextual data. * @@ -54,14 +74,19 @@ public void count(String event) { * and analyzing events later. Note: A value for a metric should be a Java primitive * (String, int, double, long float). */ - public void count(String event, Map dimensions) { + void count(String event, Map dimensions, boolean doAsync) { logger.debug("Counting new record for event: {}", event); Instant happenedAt = Instant.now(); - String serializedDimensions = serializeDimensions(dimensions); + String serializedDimensions = dimensions.isEmpty() ? null : serializeDimensions(dimensions); + + Runnable task = () -> processEvent(event, happenedAt, serializedDimensions); - service.submit(() -> processEvent(event, happenedAt, - dimensions.isEmpty() ? null : serializedDimensions)); + if (doAsync) { + service.submit(task); + } else { + task.run(); + } } private static String serializeDimensions(Map dimensions) { @@ -72,13 +97,7 @@ private static String serializeDimensions(Map dimensions) { } } - /** - * - * @param event the event to save - * @param happenedAt the moment when the event is dispatched - * @param dimensionsJson optional JSON-serialized dimensions, or null - */ - private void processEvent(String event, Instant happenedAt, @Nullable String dimensionsJson) { + void processEvent(String event, Instant happenedAt, @Nullable String dimensionsJson) { database.write(context -> context.newRecord(MetricEvents.METRIC_EVENTS) .setEvent(event) .setHappenedAt(happenedAt) @@ -86,6 +105,13 @@ private void processEvent(String event, Instant happenedAt, @Nullable String dim .insert()); } + /** + * Exposes the underlying executor service. + *

+ * Intended for test teardown only. + * + * @return the executor service backing this instance + */ public ExecutorService getExecutorService() { return service; } diff --git a/application/src/test/java/org/togetherjava/tjbot/features/MetricsTests.java b/application/src/test/java/org/togetherjava/tjbot/features/MetricsTests.java index 78ae3dac0f..c9628760c7 100644 --- a/application/src/test/java/org/togetherjava/tjbot/features/MetricsTests.java +++ b/application/src/test/java/org/togetherjava/tjbot/features/MetricsTests.java @@ -3,28 +3,17 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.togetherjava.tjbot.db.Database; import org.togetherjava.tjbot.db.generated.tables.MetricEvents; +import org.togetherjava.tjbot.db.generated.tables.records.MetricEventsRecord; import org.togetherjava.tjbot.features.analytics.Metrics; -import java.time.Duration; -import java.time.Instant; -import java.time.temporal.ChronoUnit; -import java.util.List; -import java.util.concurrent.locks.LockSupport; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.within; -import static org.junit.jupiter.api.Assertions.fail; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; final class MetricsTests { - private static final Logger logger = LoggerFactory.getLogger(MetricsTests.class); - - private static final Duration WAIT_TIMEOUT = Duration.ofSeconds(3); - private Database database; private Metrics metrics; @@ -40,79 +29,19 @@ void tearDown() { } @Test - void countInsertsSingleEvent() { + void countWithDoAsyncFalsePersists() { - final String slashPing = "slash-ping"; + String testEvent = "metrics_test_event"; - metrics.count(slashPing); + metrics.count(testEvent, false); - awaitRecords(1); + MetricEventsRecord savedRecord = + database.read(context -> context.selectFrom(MetricEvents.METRIC_EVENTS).fetchOne()); - List recordedEvents = readEventsOrderedById(); + assertNotNull(savedRecord); - assertThat(recordedEvents).as("Metrics should persist the counted event in insertion order") - .containsExactly(slashPing); - - assertThat(readLatestEventHappenedAt()) - .as("Metrics should store a recent timestamp for event '%s' (recordedEvents=%s)", - slashPing, recordedEvents) - .isNotNull() - .isCloseTo(Instant.now(), within(5, ChronoUnit.SECONDS)); + assertEquals(testEvent, savedRecord.get(MetricEvents.METRIC_EVENTS.EVENT)); + assertNull(savedRecord.get(MetricEvents.METRIC_EVENTS.DIMENSIONS)); } - private void awaitRecords(int expectedAmount) { - Instant deadline = Instant.now().plus(WAIT_TIMEOUT); - - while (Instant.now().isBefore(deadline)) { - if (readRecordCount() == expectedAmount) { - return; - } - - LockSupport.parkNanos(Duration.ofMillis(25).toNanos()); - - if (Thread.interrupted()) { - int actualCount = readRecordCount(); - - String msg = String.format( - "Interrupted while waiting for metrics writes (expectedAmount=%d, actualCount=%d, timeout=%s, events=%s)", - expectedAmount, actualCount, WAIT_TIMEOUT, readEventsOrderedById()); - - logger.warn(msg); - - fail(msg); - } - } - - int actualCount = readRecordCount(); - - List recordedEvents = readEventsOrderedById(); - - String timeoutMessage = String.format( - "Timed out waiting for metrics writes (expectedAmount=%d, actualCount=%d, timeout=%s, events=%s)", - expectedAmount, actualCount, WAIT_TIMEOUT, recordedEvents); - - logger.warn(timeoutMessage); - - assertThat(actualCount).as(timeoutMessage).isEqualTo(expectedAmount); - } - - private int readRecordCount() { - return database.read(context -> context.fetchCount(MetricEvents.METRIC_EVENTS)); - } - - private List readEventsOrderedById() { - return database.read(context -> context.select(MetricEvents.METRIC_EVENTS.EVENT) - .from(MetricEvents.METRIC_EVENTS) - .orderBy(MetricEvents.METRIC_EVENTS.ID.asc()) - .fetch(MetricEvents.METRIC_EVENTS.EVENT)); - - } - - private Instant readLatestEventHappenedAt() { - return database.read(context -> context.select(MetricEvents.METRIC_EVENTS.HAPPENED_AT) - .from(MetricEvents.METRIC_EVENTS) - .orderBy(MetricEvents.METRIC_EVENTS.ID.desc()) - .limit(1) - .fetchOne(MetricEvents.METRIC_EVENTS.HAPPENED_AT)); - } } diff --git a/application/src/test/resources/log4j2.xml b/application/src/test/resources/log4j2.xml index 9aa5025e0b..586ab0cc45 100644 --- a/application/src/test/resources/log4j2.xml +++ b/application/src/test/resources/log4j2.xml @@ -13,8 +13,6 @@ - - From 7b7a6760bd9a9607ea49b15879bbea446f88d0dc Mon Sep 17 00:00:00 2001 From: Firas Regaieg Date: Mon, 27 Apr 2026 21:18:50 +0100 Subject: [PATCH 3/3] Metrics: revert access modifier for processEvent from package-private to private --- .../java/org/togetherjava/tjbot/features/analytics/Metrics.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/application/src/main/java/org/togetherjava/tjbot/features/analytics/Metrics.java b/application/src/main/java/org/togetherjava/tjbot/features/analytics/Metrics.java index 4d4e5a503c..9e09c99f85 100644 --- a/application/src/main/java/org/togetherjava/tjbot/features/analytics/Metrics.java +++ b/application/src/main/java/org/togetherjava/tjbot/features/analytics/Metrics.java @@ -97,7 +97,7 @@ private static String serializeDimensions(Map dimensions) { } } - void processEvent(String event, Instant happenedAt, @Nullable String dimensionsJson) { + private void processEvent(String event, Instant happenedAt, @Nullable String dimensionsJson) { database.write(context -> context.newRecord(MetricEvents.METRIC_EVENTS) .setEvent(event) .setHappenedAt(happenedAt)