Merge pull request #123 from TNG/fix-assertionerror-mem-leak

Fixed memory leak in ArchRule.AssertionError in combination with JUnit 4

archunit-integration-test/src/test/java/com/tngtech/archunit/integration/ExamplesIntegrationTest.java

@@ -106,6 +106,10 @@
 import com.tngtech.archunit.testutils.ExpectedMethod;
 import com.tngtech.archunit.testutils.ExpectedTestFailures;
 import com.tngtech.archunit.testutils.MessageAssertionChain;
+import com.tngtech.archunit.testutils.ResultStoringExtension;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.DynamicTest;
 import org.junit.jupiter.api.TestFactory;
 
@@ -139,6 +143,21 @@
 
 class ExamplesIntegrationTest {
 
+ @BeforeAll
+ static void initExtension() {
+ ResultStoringExtension.enable();
+ }
+
+ @AfterEach
+ void tearDown() {
+ ResultStoringExtension.reset();
+ }
+
+ @AfterAll
+ static void disableExtension() {
+ ResultStoringExtension.disable();
+ }
+
  @TestFactory
  Stream<DynamicTest> CodingRulesTest() {
  return ExpectedTestFailures

archunit-integration-test/src/test/java/com/tngtech/archunit/integration/ExtensionIntegrationTest.java

@@ -11,54 +11,58 @@
 import com.tngtech.archunit.exampletest.extension.ExampleExtension;
 import com.tngtech.archunit.lang.ArchRule;
 import org.assertj.core.api.Condition;
-import org.junit.Test;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
 
 import static com.google.common.collect.Iterables.getOnlyElement;
 import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.noClasses;
 import static org.assertj.core.api.Assertions.assertThat;
 
-public class ExtensionIntegrationTest {
+class ExtensionIntegrationTest {
  private final JavaClasses classes = new ClassFileImporter().importPackagesOf(ClassViolatingCodingRules.class);
 
- @Test
- public void evaluation_results_are_dispatched_to_extensions() {
+ @BeforeEach
+ void setUp() {
  ExampleExtension.reset();
- try {
- ArchConfiguration.get().configureExtension(ExampleExtension.UNIQUE_IDENTIFIER)
- .setProperty("enabled", true);
+ }
+
+ @AfterEach
+ void tearDown() {
+ ExampleExtension.reset();
+ ArchConfiguration.get().configureExtension(ExampleExtension.UNIQUE_IDENTIFIER)
+ .setProperty("enabled", false);
+ }
 
- ArchRule rule = noClasses().should().haveFullyQualifiedName(ClassViolatingCodingRules.class.getName());
- checkRuleAndIgnoreFailure(classes, rule);
+ @Test
+ void evaluation_results_are_dispatched_to_extensions() {
+ ArchConfiguration.get().configureExtension(ExampleExtension.UNIQUE_IDENTIFIER)
+ .setProperty("enabled", true);
 
- assertThat(ExampleExtension.getConfigurationEvents()).hasSize(1);
- assertThat(ExampleExtension.getConfigurationEvents())
- .extracting("properties").are(containingEntry("example-prop", "exampleValue"));
+ ArchRule rule = noClasses().should().haveFullyQualifiedName(ClassViolatingCodingRules.class.getName());
+ checkRuleAndIgnoreFailure(classes, rule);
 
- EvaluatedRuleEvent event = getOnlyElement(ExampleExtension.getEvaluatedRuleEvents());
- assertThat(event.contains(rule)).as("Rule was passed").isTrue();
- assertThat(event.contains(classes)).as("Classes were passed").isTrue();
- assertThat(event.hasViolationFor(ClassViolatingCodingRules.class))
- .as("Has violation for " + ClassViolatingCodingRules.class.getSimpleName()).isTrue();
- } finally {
- ArchConfiguration.get().configureExtension(ExampleExtension.UNIQUE_IDENTIFIER).setProperty("enabled", false);
- }
+ assertThat(ExampleExtension.getConfigurationEvents()).hasSize(1);
+ assertThat(ExampleExtension.getConfigurationEvents())
+ .extracting("properties").are(containingEntry("example-prop", "exampleValue"));
+
+ EvaluatedRuleEvent event = getOnlyElement(ExampleExtension.getEvaluatedRuleEvents());
+ assertThat(event.contains(rule)).as("Rule was passed").isTrue();
+ assertThat(event.contains(classes)).as("Classes were passed").isTrue();
+ assertThat(event.hasViolationFor(ClassViolatingCodingRules.class))
+ .as("Has violation for " + ClassViolatingCodingRules.class.getSimpleName()).isTrue();
  }
 
  @Test
- public void evaluation_results_are_only_dispatched_to_enabled_extensions() {
- ExampleExtension.reset();
- try {
- ArchConfiguration.get().configureExtension(ExampleExtension.UNIQUE_IDENTIFIER)
- .setProperty("enabled", false);
+ void evaluation_results_are_only_dispatched_to_enabled_extensions() {
+ ArchConfiguration.get().configureExtension(ExampleExtension.UNIQUE_IDENTIFIER)
+ .setProperty("enabled", false);
 
-  ArchRule rule = noClasses().should().haveFullyQualifiedName(ClassViolatingCodingRules.class.getName());
-  checkRuleAndIgnoreFailure(classes, rule);
+ ArchRule rule = noClasses().should().haveFullyQualifiedName(ClassViolatingCodingRules.class.getName());
+ checkRuleAndIgnoreFailure(classes, rule);
 
- assertThat(ExampleExtension.getConfigurationEvents()).isEmpty();
- assertThat(ExampleExtension.getEvaluatedRuleEvents()).isEmpty();
- } finally {
- ArchConfiguration.get().configureExtension(ExampleExtension.UNIQUE_IDENTIFIER).setProperty("enabled", false);
- }
+ assertThat(ExampleExtension.getConfigurationEvents()).isEmpty();
+ assertThat(ExampleExtension.getEvaluatedRuleEvents()).isEmpty();
  }
 
  private static Condition<Object> containingEntry(final String propKey, final String propValue) {

archunit-integration-test/src/test/java/com/tngtech/archunit/testutils/ExpectedTestFailures.java

@@ -14,7 +14,7 @@
 
 import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableList;
-import com.tngtech.archunit.lang.ArchRule;
+import com.tngtech.archunit.lang.EvaluationResult;
 import org.junit.jupiter.api.DynamicTest;
 import org.junit.platform.runner.JUnitPlatform;
 import org.junit.runner.JUnitCore;
@@ -259,8 +259,8 @@ private TestFailure(Failure junitFailure, Throwable error) {
  this.error = error;
  }
 
- ArchRule.AssertionError getAssertionError() {
- return (ArchRule.AssertionError) error;
+ AssertionError getAssertionError() {
+ return (AssertionError) error;
  }
 
  @Override
@@ -281,13 +281,13 @@ private TestFailures(Iterable<TestFailure> failures) {
 
  void assertNoUnexpectedErrorType() {
  List<TestFailure> unexpectedFailureTypes = failures.stream()
- .filter(f -> !(f.error instanceof ArchRule.AssertionError))
+ .filter(f -> !(f.error instanceof AssertionError))
  .collect(toList());
 
  if (!unexpectedFailureTypes.isEmpty()) {
  throw new AssertionError(String.format(
  "Some failures were due to an unexpected error (i.e. not %s): %s",
- ArchRule.AssertionError.class.getName(),
+ AssertionError.class.getName(),
  unexpectedFailureTypes));
  }
  }
@@ -358,9 +358,10 @@ ExpectedViolationToAssign copy() {
  return new ExpectedViolationToAssign(failurePredicate, expectedViolation.copy(), handlingAssertion.copy());
  }
 
- Result evaluate(ArchRule.AssertionError error) {
+ Result evaluate(AssertionError error) {
  ViolationComparisonResult violationResult = expectedViolation.evaluate(error);
- HandlingAssertion.Result handlingResult = handlingAssertion.evaluate(error.getResult());
+ EvaluationResult evaluationResult = ResultStoringExtension.getEvaluationResultFor(error.getMessage());
+ HandlingAssertion.Result handlingResult = handlingAssertion.evaluate(evaluationResult);
  return new Result(violationResult, handlingResult);
  }
 

archunit-integration-test/src/test/java/com/tngtech/archunit/testutils/ResultStoringExtension.java

@@ -0,0 +1,47 @@
+package com.tngtech.archunit.testutils;
+
+import java.util.Properties;
+import java.util.concurrent.ConcurrentHashMap;
+
+import com.tngtech.archunit.ArchConfiguration;
+import com.tngtech.archunit.lang.EvaluationResult;
+import com.tngtech.archunit.lang.extension.ArchUnitExtension;
+import com.tngtech.archunit.lang.extension.EvaluatedRule;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+
+public class ResultStoringExtension implements ArchUnitExtension {
+ private static final String UNIQUE_IDENTIFIER = "archunit-result-storing-extension";
+
+ private static final ConcurrentHashMap<String, EvaluationResult> storedResults = new ConcurrentHashMap<>();
+
+ @Override
+ public String getUniqueIdentifier() {
+ return UNIQUE_IDENTIFIER;
+ }
+
+ @Override
+ public void configure(Properties properties) {
+ }
+
+ @Override
+ public void handle(EvaluatedRule evaluatedRule) {
+ storedResults.put(evaluatedRule.getResult().getFailureReport().toString(), evaluatedRule.getResult());
+ }
+
+ public static void reset() {
+ storedResults.clear();
+ }
+
+ static EvaluationResult getEvaluationResultFor(String errorMessage) {
+ return checkNotNull(storedResults.get(errorMessage), "No result was recorded for error message: %s", errorMessage);
+ }
+
+ public static void enable() {
+ ArchConfiguration.get().configureExtension(UNIQUE_IDENTIFIER).setProperty("enabled", true);
+ }
+
+ public static void disable() {
+ ArchConfiguration.get().configureExtension(UNIQUE_IDENTIFIER).setProperty("enabled", false);
+ }
+}

archunit-integration-test/src/test/resources/META-INF/services/com.tngtech.archunit.lang.extension.ArchUnitExtension

@@ -0,0 +1 @@
+com.tngtech.archunit.testutils.ResultStoringExtension

archunit/src/main/java/com/tngtech/archunit/lang/ArchRule.java

@@ -89,8 +89,7 @@ public static void assertNoViolation(EvaluationResult result) {
  Set<Pattern> patterns = readPatternsFrom(ARCHUNIT_IGNORE_PATTERNS_FILE_NAME);
  report = report.filter(notMatchedByAny(patterns));
  if (!report.isEmpty()) {
- String message = report.toString();
- throw new AssertionError(message, result);
+ throw new AssertionError(report.toString());
  }
  }
 
@@ -261,18 +260,4 @@ public String toString() {
  }
  }
  }
-
- final class AssertionError extends java.lang.AssertionError {
- private final EvaluationResult result;
-
- private AssertionError(String message, EvaluationResult result) {
- super(message);
- this.result = result;
- }
-
- @PublicAPI(usage = ACCESS)
- public EvaluationResult getResult() {
- return result;
- }
- }
 }