This is an automated email from the ASF dual-hosted git repository. ggregory pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/commons-lang.git
commit b62fc004d1a60757090ba9a177202eed0db57cc0 Author: Gary Gregory <[email protected]> AuthorDate: Mon Dec 9 07:24:17 2024 -0500 Add some tests that use reflection Still need to find a way to check for memory retention beyond what AbstractLangTest.after() does --- .../lang3/builder/ReflectionDiffBuilder.java | 1 - .../org/apache/commons/lang3/AbstractLangTest.java | 1 + .../EqualsBuilderReflectJreImplementationTest.java | 22 ++++++-- .../HashCodeBuilderAndEqualsBuilderTest.java | 10 ++++ .../lang3/builder/ReflectionDiffBuilderTest.java | 18 +++++++ .../commons/lang3/builder/TestClassBuilder.java | 60 ++++++++++++++++++++++ 6 files changed, 108 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/apache/commons/lang3/builder/ReflectionDiffBuilder.java b/src/main/java/org/apache/commons/lang3/builder/ReflectionDiffBuilder.java index cef3b2892..8e4d0970f 100644 --- a/src/main/java/org/apache/commons/lang3/builder/ReflectionDiffBuilder.java +++ b/src/main/java/org/apache/commons/lang3/builder/ReflectionDiffBuilder.java @@ -212,7 +212,6 @@ public class ReflectionDiffBuilder<T> implements Builder<DiffResult<T>> { if (getLeft().equals(getRight())) { return diffBuilder.build(); } - appendFields(getLeft().getClass()); return diffBuilder.build(); } diff --git a/src/test/java/org/apache/commons/lang3/AbstractLangTest.java b/src/test/java/org/apache/commons/lang3/AbstractLangTest.java index ca9ff1b49..b6d46a158 100644 --- a/src/test/java/org/apache/commons/lang3/AbstractLangTest.java +++ b/src/test/java/org/apache/commons/lang3/AbstractLangTest.java @@ -32,6 +32,7 @@ public class AbstractLangTest { @AfterEach public void after() { assertTrue(ToStringStyle.getRegistry().isEmpty(), "Expected null, actual: " + ToStringStyle.getRegistry()); + // TODO Do more to make sure memory is not retained, maybe like Log4j checks for it. } } diff --git a/src/test/java/org/apache/commons/lang3/builder/EqualsBuilderReflectJreImplementationTest.java b/src/test/java/org/apache/commons/lang3/builder/EqualsBuilderReflectJreImplementationTest.java index a8001d377..d5223e1be 100644 --- a/src/test/java/org/apache/commons/lang3/builder/EqualsBuilderReflectJreImplementationTest.java +++ b/src/test/java/org/apache/commons/lang3/builder/EqualsBuilderReflectJreImplementationTest.java @@ -42,10 +42,13 @@ import java.time.temporal.TemporalAccessor; import java.time.temporal.TemporalAmount; import java.time.temporal.TemporalField; import java.time.temporal.TemporalUnit; +import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.function.Supplier; import org.apache.commons.lang3.AbstractLangTest; +import org.apache.commons.lang3.stream.IntStreams; import org.junit.jupiter.api.Test; /** @@ -90,13 +93,14 @@ public class EqualsBuilderReflectJreImplementationTest extends AbstractLangTest } } - static class MyClass { + static class MyClass implements Cloneable { private final MyCharSequence charSequence; private final MyTemporal temporal; private final MyTemporalAccessor temporalAccessor; private final MyTemporalAmount temporalAmount; private final Object[] objects; + private final List<Supplier<?>> list = new ArrayList<>(); MyClass(final MyCharSequence charSequence, final MyTemporal temporal, final MyTemporalAccessor temporalAccessor, final MyTemporalAmount temporalAmount) { @@ -117,6 +121,7 @@ public class EqualsBuilderReflectJreImplementationTest extends AbstractLangTest localDate, HijrahDate.from(localDate), JapaneseDate.from(localDate), MinguoDate.from(localDate), ThaiBuddhistDate.from(localDate), localDate, localTime, localDateTime, offsetDateTime, OffsetTime.of(localTime, zoneOffset), Year.of(value), YearMonth.of(value, value), ZonedDateTime.of(localDateTime, zoneOffset), zoneOffset, ZoneId.of(zoneOffset.getId()) }; + IntStreams.range(100).forEach(i -> list.add(() -> charSequence)); } @Override @@ -178,6 +183,7 @@ public class EqualsBuilderReflectJreImplementationTest extends AbstractLangTest } } + static class MyTemporalAccessor implements TemporalAccessor { private final String string; @@ -256,9 +262,9 @@ public class EqualsBuilderReflectJreImplementationTest extends AbstractLangTest @Test public void testRecursive() { - final MyClass o1 = new MyClass(new MyCharSequence("1"), new MyTemporal("2"), new MyTemporalAccessor("3"), new MyTemporalAmount("4")); + final MyClass o1 = new MyClass(new MyCharSequence("1"), new MyTemporal("2"), new MyTemporalAccessor("3"), new MyTemporalAmount("4")); // This gives you different instances of MyTemporalAccessor for 1 (and 2) that should be equals by reflection. - final MyClass o1Bis = new MyClass(new MyCharSequence("1"), new MyTemporal("2"), new MyTemporalAccessor("3"), new MyTemporalAmount("4")); + final MyClass o1Bis = new MyClass(new MyCharSequence("1"), new MyTemporal("2"), new MyTemporalAccessor("3"), new MyTemporalAmount("4")); final MyClass o2 = new MyClass(new MyCharSequence("5"), new MyTemporal("6"), new MyTemporalAccessor("7"), new MyTemporalAmount("8")); final MyClass o2Bis = new MyClass(new MyCharSequence("5"), new MyTemporal("6"), new MyTemporalAccessor("7"), new MyTemporalAmount("8")); // MyTemporal @@ -276,4 +282,14 @@ public class EqualsBuilderReflectJreImplementationTest extends AbstractLangTest assertFalse(new EqualsBuilder().setTestRecursive(true).append(o2, o1).isEquals()); } + @Test + public void testRetention() throws Exception { + // The following should not retain memory. + for (int i = 0; i < Integer.getInteger("testRetention", 10_000); i++) { + final Class<?> clazz = TestClassBuilder.defineSimpleClass(getClass().getPackage().getName(), i); + assertTrue(new EqualsBuilder().setTestRecursive(true).append(clazz.newInstance(), clazz.newInstance()).isEquals()); + } + // some retention is checked in super's after(). + } + } diff --git a/src/test/java/org/apache/commons/lang3/builder/HashCodeBuilderAndEqualsBuilderTest.java b/src/test/java/org/apache/commons/lang3/builder/HashCodeBuilderAndEqualsBuilderTest.java index 5fa03eb38..8de3b010f 100644 --- a/src/test/java/org/apache/commons/lang3/builder/HashCodeBuilderAndEqualsBuilderTest.java +++ b/src/test/java/org/apache/commons/lang3/builder/HashCodeBuilderAndEqualsBuilderTest.java @@ -110,6 +110,7 @@ public class HashCodeBuilderAndEqualsBuilderTest extends AbstractLangTest { new SubAllTransientFixture(2, 'c', "Test", (short) 2, "Same"), new SubAllTransientFixture(2, 'c', "Test", (short) 2, "Same"), testTransients); + } @Test @@ -133,4 +134,13 @@ public class HashCodeBuilderAndEqualsBuilderTest extends AbstractLangTest { testInteger(true); } + @Test + public void testRetention() throws Exception { + // The following should not retain memory. + for (int i = 0; i < Integer.getInteger("testRecursive", 10_000); i++) { + final Class<?> clazz = TestClassBuilder.defineSimpleClass(getClass().getPackage().getName(), i); + assertEqualsAndHashCodeContract(clazz.newInstance(), clazz.newInstance(), false); + } + } + } diff --git a/src/test/java/org/apache/commons/lang3/builder/ReflectionDiffBuilderTest.java b/src/test/java/org/apache/commons/lang3/builder/ReflectionDiffBuilderTest.java index 04b37e7be..86286627a 100644 --- a/src/test/java/org/apache/commons/lang3/builder/ReflectionDiffBuilderTest.java +++ b/src/test/java/org/apache/commons/lang3/builder/ReflectionDiffBuilderTest.java @@ -37,6 +37,7 @@ public class ReflectionDiffBuilderTest extends AbstractLangTest { @SuppressWarnings("unused") private static class TypeTestClass implements Diffable<TypeTestClass> { + private static int staticField; private final ToStringStyle style = SHORT_STYLE; private final boolean booleanField = true; @@ -159,6 +160,7 @@ public class ReflectionDiffBuilderTest extends AbstractLangTest { final String[] excludeFieldNames = reflectionDiffBuilder.getExcludeFieldNames(); assertNotNull(excludeFieldNames); assertEquals(0, excludeFieldNames.length); + assertNotNull(reflectionDiffBuilder.build()); } @Test @@ -171,6 +173,7 @@ public class ReflectionDiffBuilderTest extends AbstractLangTest { final String[] excludeFieldNames = reflectionDiffBuilder.getExcludeFieldNames(); assertNotNull(excludeFieldNames); assertEquals(0, excludeFieldNames.length); + assertNotNull(reflectionDiffBuilder.build()); } @Test @@ -189,6 +192,7 @@ public class ReflectionDiffBuilderTest extends AbstractLangTest { assertNotNull(excludeFieldNames); assertEquals(1, excludeFieldNames.length); assertEquals("charField", excludeFieldNames[0]); + assertNotNull(reflectionDiffBuilder.build()); } @Test @@ -202,6 +206,7 @@ public class ReflectionDiffBuilderTest extends AbstractLangTest { assertNotNull(excludeFieldNames); assertEquals(1, excludeFieldNames.length); assertEquals("charField", excludeFieldNames[0]); + assertNotNull(reflectionDiffBuilder.build()); } @Test @@ -212,6 +217,19 @@ public class ReflectionDiffBuilderTest extends AbstractLangTest { assertEquals(0, firstObject.diffDeprecated(secondObject).getNumberOfDiffs()); } + @Test + public void testRetention() throws Exception { + // The following should not retain memory. + for (int i = 0; i < Integer.getInteger("testRecursive", 10_000); i++) { + final Class<?> clazz = TestClassBuilder.defineSimpleClass(getClass().getPackage().getName(), i); + final Object firstObject = clazz.newInstance(); + final Object secondObject = clazz.newInstance(); + final ReflectionDiffBuilder<Object> reflectionDiffBuilder = new ReflectionDiffBuilder<>(firstObject, secondObject, SHORT_STYLE); + assertNotNull(reflectionDiffBuilder.build()); + } + } + + @Test public void testNoDifferencesDiffExcludeAnnotatedField() { final TypeTestClass firstObject = new TypeTestClass(); diff --git a/src/test/java/org/apache/commons/lang3/builder/TestClassBuilder.java b/src/test/java/org/apache/commons/lang3/builder/TestClassBuilder.java new file mode 100644 index 000000000..09760d938 --- /dev/null +++ b/src/test/java/org/apache/commons/lang3/builder/TestClassBuilder.java @@ -0,0 +1,60 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.commons.lang3.builder; + +import org.objectweb.asm.ClassWriter; +import org.objectweb.asm.MethodVisitor; +import org.objectweb.asm.Opcodes; + +/** + * Builds classes dynamically for tests. + */ +public class TestClassBuilder { + + /** + * Extends {@link ClassLoader} to make {@link ClassLoader#defineClass(String, byte[])} public. + */ + static final class DynamicClassLoader extends ClassLoader { + public Class<?> defineClass(final String name, final byte[] b) { + return defineClass(name, b, 0, b.length); + } + } + + /** + * Defines the simplest possible class. + * + * @param name The class name. + * @return The new class. + */ + static Class<?> defineSimpleClass(final String name) { + final ClassWriter classWriter = new ClassWriter(ClassWriter.COMPUTE_FRAMES | ClassWriter.COMPUTE_MAXS); + classWriter.visit(Opcodes.V1_8, Opcodes.ACC_PUBLIC, name, null, "java/lang/Object", new String[] {}); + final MethodVisitor ctor = classWriter.visitMethod(Opcodes.ACC_PUBLIC, "<init>", "()V", null, null); + ctor.visitCode(); + ctor.visitVarInsn(Opcodes.ALOAD, 0); + ctor.visitMethodInsn(Opcodes.INVOKESPECIAL, "java/lang/Object", "<init>", "()V", false); + ctor.visitInsn(Opcodes.RETURN); + ctor.visitMaxs(1, 1); + return new DynamicClassLoader().defineClass(name.replace('/', '.'), classWriter.toByteArray()); + } + + static Class<?> defineSimpleClass(final String packageName, final int i) { + return defineSimpleClass(packageName.replace('.', '/') + "/C" + i); + } + +}
