From 9fa1e843f620e51536f8cf8d1ab5883f7610c29a Mon Sep 17 00:00:00 2001 From: Daniel Flassak Date: Fri, 23 Sep 2022 17:10:14 +0200 Subject: [PATCH] remove key-value assertion again --- .github/workflows/build.yml | 2 - README.md | 26 +--- pom.xml | 7 - project-without-logstash/pom.xml | 87 ------------ .../test/java/com/acme/LogCaptureTest.java | 34 ----- .../logcapture/ExpectedKeyValue.java | 83 ----------- .../ExpectedKeyValueLogstashDelegate.java | 36 ----- .../logcapture/ExpectedLoggerName.java | 2 +- .../logcapture/ExpectedMarker.java | 2 +- .../logcapture/ExpectedMdcEntry.java | 3 +- .../com/example/app/LogstashKeyValueTest.java | 130 ------------------ .../logcapture/ExpectedKeyValueUnitTest.java | 16 --- .../logcapture/LogCaptureArchTest.java | 10 +- 13 files changed, 15 insertions(+), 423 deletions(-) delete mode 100644 project-without-logstash/pom.xml delete mode 100644 project-without-logstash/src/test/java/com/acme/LogCaptureTest.java delete mode 100644 src/main/java/de/dm/infrastructure/logcapture/ExpectedKeyValue.java delete mode 100644 src/main/java/de/dm/infrastructure/logcapture/ExpectedKeyValueLogstashDelegate.java delete mode 100644 src/test/java/com/example/app/LogstashKeyValueTest.java delete mode 100644 src/test/java/de/dm/infrastructure/logcapture/ExpectedKeyValueUnitTest.java diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 0b914a4..42524c3 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -9,5 +9,3 @@ jobs: with: COMMAND: > mvn --batch-mode -Dmaven.compiler.showDeprecation=true -Dmaven.compiler.showWarnings=true -Dproject.version=0.0.0-SNAPSHOT clean install - && cd project-without-logstash - && mvn --batch-mode -Dlog-capture.version=0.0.0-SNAPSHOT clean verify diff --git a/README.md b/README.md index 94114b3..18b5221 100644 --- a/README.md +++ b/README.md @@ -40,7 +40,6 @@ logCapture.assertLoggedInOrder( * [Exceptions](#exceptions) * [Markers](#markers) * [Logger name](#logger-name) - * [Key-Value (from Logstash)](#key-value-from-logstash) * [Examples](#examples) * [Unit Test Example:](#unit-test-example) * [Integration Test Example:](#integration-test-example) @@ -50,6 +49,7 @@ logCapture.assertLoggedInOrder( * [Cucumber example](#cucumber-example) * [Cucumber feature file](#cucumber-feature-file) * [Changes](#changes) + * [3.6.0](#360) * [3.5.0](#350) * [3.4.1](#341) * [3.4.0](#340) @@ -141,26 +141,6 @@ log.info("did something"); logCapture.info("did something", logger("com.acme.foo")); ``` -#### Key-Value (from Logstash) - -**Note that** this will only work if logstash-logback-encoder is in your classpath - log-capture does not depend on it, so you need to add it manually if you intend to use it. - -```java -import static de.dm.infrastructure.logcapture.ExpectedKeyValue.keyValue; - -... - -log.info("hello", - StructuredArguments.keyValue("myString", "hello"), - StructuredArguments.keyValue("myNumber", 42) -); - -logCapture.assertLogged(info("hello", - keyValue("myString", "hello"), - keyValue("myNumber", 42)) -); -``` - ### Examples #### Unit Test Example: @@ -328,6 +308,10 @@ And with MDC logging context ## Changes +### 3.6.0 + +* Removed ExpectedKeyValue again due to an [API change in Logstash without a workaround](https://github.com/logfellow/logstash-logback-encoder/issues/788) + ### 3.5.0 * Added new Log Event Matcher: `ExpectedKeyValue.keyValue(...)` to assert `StructuredArguments.keyValue(...)` from Logstash diff --git a/pom.xml b/pom.xml index 8dabf98..e42f49a 100644 --- a/pom.xml +++ b/pom.xml @@ -34,7 +34,6 @@ 1.8 1.18.22 1.2.11 - 7.0.1 5.8.2 UTF-8 @@ -77,12 +76,6 @@ ${lombok.version} provided - - net.logstash.logback - logstash-logback-encoder - ${logstash-logback-encoder.version} - provided - diff --git a/project-without-logstash/pom.xml b/project-without-logstash/pom.xml deleted file mode 100644 index 4813890..0000000 --- a/project-without-logstash/pom.xml +++ /dev/null @@ -1,87 +0,0 @@ - - - 4.0.0 - - de.acme - test-project-without-logstash - 0.0.1-SNAPSHOT - - - - MIT - https://opensource.org/licenses/MIT - repo - - - - - 1.8 - UTF-8 - 3.4.1-SNAPSHOT - - 1.18.22 - 1.2.11 - 5.8.2 - 3.22.0 - - 2.22.2 - 3.9.0 - - - - - org.projectlombok - lombok - ${lombok.version} - provided - - - - - org.junit.jupiter - junit-jupiter-api - ${junit.version} - test - - - org.junit.jupiter - junit-jupiter-engine - ${junit.version} - test - - - de.dm.infrastructure - log-capture - ${log-capture.version} - - - org.assertj - assertj-core - ${assertj-core.version} - test - - - - - - - org.apache.maven.plugins - maven-compiler-plugin - ${maven-compiler-plugin.version} - - ${java.version} - ${java.version} - ${encoding} - true - true - - - - org.apache.maven.plugins - maven-surefire-plugin - ${maven-surefire-plugin.version} - - - - diff --git a/project-without-logstash/src/test/java/com/acme/LogCaptureTest.java b/project-without-logstash/src/test/java/com/acme/LogCaptureTest.java deleted file mode 100644 index 3416887..0000000 --- a/project-without-logstash/src/test/java/com/acme/LogCaptureTest.java +++ /dev/null @@ -1,34 +0,0 @@ -package com.acme; - -import de.dm.infrastructure.logcapture.LogCapture; -import lombok.extern.slf4j.Slf4j; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.RegisterExtension; - -import static de.dm.infrastructure.logcapture.ExpectedKeyValue.keyValue; -import static de.dm.infrastructure.logcapture.LogExpectation.info; -import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.jupiter.api.Assertions.assertThrows; - -@Slf4j -public class LogCaptureTest { - @RegisterExtension - LogCapture logCapture = LogCapture.forCurrentPackage(); - - @Test - void assertionWorksDespiteLogstashNotBeingInTheClasspath() { - log.info("hello"); - logCapture.assertLogged(info("hello")); - } - - @Test - void errorMessageForKeyValueAssertion() { - log.info("hello"); - - IllegalArgumentException thrown = assertThrows(IllegalArgumentException.class, () -> - logCapture.assertLogged(info("hello", keyValue("key", "value")))); - - assertThat(thrown).hasMessage("keyValue cannot be used for log assertions if logstash-logback-encoder " + - "that provides StructuredArguments.keyValue(...) is not in the classpath."); - } -} diff --git a/src/main/java/de/dm/infrastructure/logcapture/ExpectedKeyValue.java b/src/main/java/de/dm/infrastructure/logcapture/ExpectedKeyValue.java deleted file mode 100644 index 7ce60dd..0000000 --- a/src/main/java/de/dm/infrastructure/logcapture/ExpectedKeyValue.java +++ /dev/null @@ -1,83 +0,0 @@ -package de.dm.infrastructure.logcapture; - -import lombok.AccessLevel; -import lombok.RequiredArgsConstructor; - -import static java.lang.String.format; - -/** - * define expected StructuredArgument.keyValue(...) from logstash with this matcher - */ -@RequiredArgsConstructor(access = AccessLevel.PRIVATE) -public class ExpectedKeyValue implements LogEventMatcher { - static final String LOGSTASH_MARKER_CLASS = "net.logstash.logback.marker.SingleFieldAppendingMarker"; - private final String expectedKey; - private final String expectedValue; - - @Override - public boolean matches(LoggedEvent loggedEvent) { - failIfLogstashIsNotInClasspath(); - return ExpectedKeyValueLogstashDelegate.matches(loggedEvent, expectedKey, expectedValue); - } - - @Override - public String getNonMatchingErrorMessage(LoggedEvent loggedEvent) { - failIfLogstashIsNotInClasspath(); - return ExpectedKeyValueLogstashDelegate.getNonMatchingErrorMessage(loggedEvent, expectedKey, expectedValue); - } - - @Override - public String getMatcherTypeDescription() { - return "key-value content"; - } - - @Override - public String getMatcherDetailDescription() { - return format("keyValue content with key: \"%s\" and value: \"%s\"", expectedKey, expectedValue); - } - - /** - * use this in a log expectation to verify that something has been logged with a keyValue argument from logstashg's StructuredArgument - * - * @param expectedKey expected key - * @param expectedValue expected value - * - * @return expected keyValue to use in log expectation - */ - public static ExpectedKeyValue keyValue(String expectedKey, String expectedValue) { - return new ExpectedKeyValue(expectedKey, expectedValue); - } - - /** - * use this in a log expectation to verify that something has been logged with a keyValue argument from logstashg's StructuredArgument - * - * @param expectedKey expected key - * @param expectedValue expected value - * - * @return expected keyValue to use in log expectation - */ - public static ExpectedKeyValue keyValue(String expectedKey, int expectedValue) { - return new ExpectedKeyValue(expectedKey, String.valueOf(expectedValue)); - } - - /** - * use this in a log expectation to verify that something has been logged with a keyValue argument from logstashg's StructuredArgument - * - * @param expectedKey expected key - * @param expectedValue expected value - * - * @return expected keyValue to use in log expectation - */ - public static ExpectedKeyValue keyValue(String expectedKey, long expectedValue) { - return new ExpectedKeyValue(expectedKey, String.valueOf(expectedValue)); - } - - private void failIfLogstashIsNotInClasspath() { - try { - Class.forName(LOGSTASH_MARKER_CLASS, false, getClass().getClassLoader()); - } catch (ClassNotFoundException e) { - throw new IllegalArgumentException("keyValue cannot be used for log assertions if " + - "logstash-logback-encoder that provides StructuredArguments.keyValue(...) is not in the classpath."); - } - } -} diff --git a/src/main/java/de/dm/infrastructure/logcapture/ExpectedKeyValueLogstashDelegate.java b/src/main/java/de/dm/infrastructure/logcapture/ExpectedKeyValueLogstashDelegate.java deleted file mode 100644 index fb39f26..0000000 --- a/src/main/java/de/dm/infrastructure/logcapture/ExpectedKeyValueLogstashDelegate.java +++ /dev/null @@ -1,36 +0,0 @@ -package de.dm.infrastructure.logcapture; - -import net.logstash.logback.marker.SingleFieldAppendingMarker; - -import java.util.Arrays; -import java.util.stream.Collectors; -import java.util.stream.Stream; - -import static java.lang.String.format; -import static java.lang.System.lineSeparator; - -// this is a delegate accessing net.logstash so that ExpectedKeyValue can fail gracefully -// if logstash is not in the classpath -final class ExpectedKeyValueLogstashDelegate { - private ExpectedKeyValueLogstashDelegate() { - } - - static boolean matches(LoggedEvent loggedEvent, String expectedKey, String expectedValue) { - return Arrays.stream(loggedEvent.getArgumentArray()) - .flatMap(argument -> argument instanceof SingleFieldAppendingMarker ? Stream.of((SingleFieldAppendingMarker) argument) : Stream.empty()) - .anyMatch(marker -> expectedValue.equals(marker.getFieldValue()) && marker.getFieldName().equals(expectedKey)); - } - - static String getNonMatchingErrorMessage(LoggedEvent loggedEvent, String expectedKey, String expectedValue) { - String actualKeyValue = Arrays.stream(loggedEvent.getArgumentArray()) - .flatMap(argument -> argument instanceof SingleFieldAppendingMarker ? Stream.of((SingleFieldAppendingMarker) argument) : Stream.empty()) - .map(marker -> format(" key: \"%s\", value: \"%s\"", marker.getFieldName(), marker.getFieldValue())) - .collect(Collectors.joining(lineSeparator())); - - return format(" expected key-value content: key: \"%s\", value: \"%s\"", expectedKey, expectedValue) + - lineSeparator() + - (actualKeyValue.isEmpty() ? " but no key-value content was found" : " actual key-value content:" + lineSeparator() + actualKeyValue); - - } - -} diff --git a/src/main/java/de/dm/infrastructure/logcapture/ExpectedLoggerName.java b/src/main/java/de/dm/infrastructure/logcapture/ExpectedLoggerName.java index 3b860f3..ee94b5a 100644 --- a/src/main/java/de/dm/infrastructure/logcapture/ExpectedLoggerName.java +++ b/src/main/java/de/dm/infrastructure/logcapture/ExpectedLoggerName.java @@ -12,7 +12,7 @@ public final class ExpectedLoggerName implements LogEventMatcher { private final Pattern expectedName; private final String inputRegex; - ExpectedLoggerName(String loggerNameRegex) { + private ExpectedLoggerName(String loggerNameRegex) { inputRegex = loggerNameRegex; expectedName = Pattern.compile(".*" + loggerNameRegex + ".*"); } diff --git a/src/main/java/de/dm/infrastructure/logcapture/ExpectedMarker.java b/src/main/java/de/dm/infrastructure/logcapture/ExpectedMarker.java index f032fce..cb2a8bd 100644 --- a/src/main/java/de/dm/infrastructure/logcapture/ExpectedMarker.java +++ b/src/main/java/de/dm/infrastructure/logcapture/ExpectedMarker.java @@ -9,7 +9,7 @@ public final class ExpectedMarker implements LogEventMatcher { private final String expectedName; - ExpectedMarker(String expectedName) { + private ExpectedMarker(String expectedName) { this.expectedName = expectedName; } diff --git a/src/main/java/de/dm/infrastructure/logcapture/ExpectedMdcEntry.java b/src/main/java/de/dm/infrastructure/logcapture/ExpectedMdcEntry.java index 14b86ce..e295300 100644 --- a/src/main/java/de/dm/infrastructure/logcapture/ExpectedMdcEntry.java +++ b/src/main/java/de/dm/infrastructure/logcapture/ExpectedMdcEntry.java @@ -7,11 +7,12 @@ import static java.lang.String.format; import static java.lang.System.lineSeparator; +import static lombok.AccessLevel.PRIVATE; /** * define expected MDC entries with this */ -@RequiredArgsConstructor +@RequiredArgsConstructor(access = PRIVATE) public final class ExpectedMdcEntry implements LogEventMatcher { private final String key; diff --git a/src/test/java/com/example/app/LogstashKeyValueTest.java b/src/test/java/com/example/app/LogstashKeyValueTest.java deleted file mode 100644 index d48a480..0000000 --- a/src/test/java/com/example/app/LogstashKeyValueTest.java +++ /dev/null @@ -1,130 +0,0 @@ -package com.example.app; - -import de.dm.infrastructure.logcapture.LogCapture; -import lombok.extern.slf4j.Slf4j; -import net.logstash.logback.argument.StructuredArguments; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.RegisterExtension; - -import static de.dm.infrastructure.logcapture.ExpectedKeyValue.keyValue; -import static de.dm.infrastructure.logcapture.LogExpectation.info; -import static java.lang.System.lineSeparator; -import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.jupiter.api.Assertions.assertThrows; - -@SuppressWarnings("java:S5778") //this rule does not increase the clarity of these tests -@Slf4j -class LogstashKeyValueTest { - @RegisterExtension - LogCapture logCapture = LogCapture.forCurrentPackage(); - - @Test - void worksWithString() { - log.info("hello", StructuredArguments.keyValue("myKey", "myValue")); - - logCapture.assertLogged(info("hello", keyValue("myKey", "myValue"))); - } - - @Test - void failsWithString() { - log.info("hello", StructuredArguments.keyValue("myKey", "actualValue")); - - AssertionError assertionError = assertThrows(AssertionError.class, () -> - logCapture.assertLogged(info("hello", keyValue("myKey", "expectedValue")))); - - assertThat(assertionError).hasMessageFindingMatch( - "Expected log message has occurred, but never with the expected key-value content: Level: INFO, Regex: \"hello\"" + ".*" + - " expected key-value content: key: \"myKey\", value: \"expectedValue\"" + ".*" + - " actual key-value content:" + ".*" + - " key: \"myKey\", value: \"actualValue\""); - } - - @Test - void worksWithInteger() { - log.info("hello", StructuredArguments.keyValue("myKey", 100000)); - - logCapture.assertLogged(info("hello", keyValue("myKey", 100000))); - } - - @Test - void failsWithInteger() { - log.info("hello", StructuredArguments.keyValue("myKey", 100001)); - - AssertionError assertionError = assertThrows(AssertionError.class, () -> - logCapture.assertLogged(info("hello", keyValue("myKey", 100000)))); - - assertThat(assertionError).hasMessageFindingMatch( - "Expected log message has occurred, but never with the expected key-value content: Level: INFO, Regex: \"hello\"" + ".*" + - " expected key-value content: key: \"myKey\", value: \"100000\"" + ".*" + - " actual key-value content:" + ".*" + - " key: \"myKey\", value: \"100001\""); - } - - @Test - void failsWithIntegerThatIsNoInteger() { - log.info("hello", StructuredArguments.keyValue("myKey", "not an int")); - - AssertionError assertionError = assertThrows(AssertionError.class, () -> - logCapture.assertLogged(info("hello", keyValue("myKey", 100000)))); - - assertThat(assertionError).hasMessageFindingMatch( - "Expected log message has occurred, but never with the expected key-value content: Level: INFO, Regex: \"hello\"" + ".*" + - " expected key-value content: key: \"myKey\", value: \"100000\"" + ".*" + - " actual key-value content:" + ".*" + - " key: \"myKey\", value: \"not an int\""); - } - - - @Test - void worksWithLong() { - log.info("hello", StructuredArguments.keyValue("myKey", 1000000000000L)); - - logCapture.assertLogged(info("hello", keyValue("myKey", 1000000000000L))); - } - - @Test - void failsWithLong() { - log.info("hello", StructuredArguments.keyValue("myKey", 1000000000001L)); - - AssertionError assertionError = assertThrows(AssertionError.class, () -> - logCapture.assertLogged(info("hello", keyValue("myKey", 1000000000000L)))); - - assertThat(assertionError).hasMessageFindingMatch( - "Expected log message has occurred, but never with the expected key-value content: Level: INFO, Regex: \"hello\"" + ".*" + - " expected key-value content: key: \"myKey\", value: \"1000000000000\"" + ".*" + - " actual key-value content:" + ".*" + - " key: \"myKey\", value: \"1000000000001\""); - } - - @Test - void failsWithLongThatIsNoLong() { - log.info("hello", StructuredArguments.keyValue("myKey", "not a long")); - - AssertionError assertionError = assertThrows(AssertionError.class, () -> - logCapture.assertLogged(info("hello", keyValue("myKey", 1000000000000L)))); - - assertThat(assertionError).hasMessageFindingMatch( - "Expected log message has occurred, but never with the expected key-value content: Level: INFO, Regex: \"hello\"" + ".*" + - " expected key-value content: key: \"myKey\", value: \"1000000000000\"" + ".*" + - " actual key-value content:" + ".*" + - " key: \"myKey\", value: \"not a long\""); - } - - @Test - void assertNotLoggedSucceeds() { - log.info("info", StructuredArguments.keyValue("key", "actualValue")); - - logCapture.assertNotLogged(info("info", keyValue("key", "forbiddenValue"))); - } - - @Test - void assertNotLoggedFailsWithProperMessage() { - log.info("info", StructuredArguments.keyValue("key", "forbiddenValue")); - - AssertionError actual = assertThrows(AssertionError.class, () -> - logCapture.assertNotLogged(info("info", keyValue("key", "forbiddenValue")))); - - assertThat(actual).hasMessage("Found a log message that should not be logged: Level: INFO, Regex: \"info\", with matchers:" + - lineSeparator() + " keyValue content with key: \"key\" and value: \"forbiddenValue\""); - } -} diff --git a/src/test/java/de/dm/infrastructure/logcapture/ExpectedKeyValueUnitTest.java b/src/test/java/de/dm/infrastructure/logcapture/ExpectedKeyValueUnitTest.java deleted file mode 100644 index 236b9f3..0000000 --- a/src/test/java/de/dm/infrastructure/logcapture/ExpectedKeyValueUnitTest.java +++ /dev/null @@ -1,16 +0,0 @@ -package de.dm.infrastructure.logcapture; - -import net.logstash.logback.marker.SingleFieldAppendingMarker; -import org.junit.jupiter.api.Test; - -import static org.assertj.core.api.Assertions.assertThat; - -class ExpectedKeyValueUnitTest { - @Test - @SuppressWarnings("squid:S3415") - // assertion arguments are in the right order, despite Sonar thinking otherwise - void logstashMarkerClassCanonicalNameIsCorrect() { - assertThat(ExpectedKeyValue.LOGSTASH_MARKER_CLASS).isEqualTo(SingleFieldAppendingMarker.class.getCanonicalName()); - } - -} diff --git a/src/test/java/de/dm/infrastructure/logcapture/LogCaptureArchTest.java b/src/test/java/de/dm/infrastructure/logcapture/LogCaptureArchTest.java index ddfabe4..6787d14 100644 --- a/src/test/java/de/dm/infrastructure/logcapture/LogCaptureArchTest.java +++ b/src/test/java/de/dm/infrastructure/logcapture/LogCaptureArchTest.java @@ -10,10 +10,12 @@ @AnalyzeClasses(packagesOf = LogCapture.class, importOptions = DoNotIncludeTests.class) public class LogCaptureArchTest { @ArchTest - static ArchRule logstashIsOnlyUsedInDelegate = classes() + static ArchRule niceApiForExpectations = classes() .that() - .doNotHaveFullyQualifiedName(ExpectedKeyValueLogstashDelegate.class.getCanonicalName()) + .haveSimpleNameStartingWith("Expected") + .and() + .haveSimpleNameNotStartingWith("ExpectedException")// because somehow ArchUnit has hiccups in this class, although it only has private constructors .should() - .onlyDependOnClassesThat() - .resideOutsideOfPackage("net.logstash.logback.."); + .haveOnlyPrivateConstructors() + .because("log expectations should have easy-to-read builders instead of public constructors"); }