Skip to content

Commit

Permalink
test: add test that verifies test element does not change after save+…
Browse files Browse the repository at this point in the history 
…load roundtrip

ProxyControl initialized collection properties with HashSet which caused
CollectionProperty.equals to return false.
CollectionProperty is normalized to ArrayList on save, so we should not pass
HashSet to the CollectionProperty in the first place.
  • Loading branch information
@vlsi
vlsi committed Jun 28, 2023
1 parent 6cc553f commit 1e45414
Show file tree
Hide file tree
Showing 5 changed files with 101 additions and 8 deletions.
14 changes: 10 additions & 4 deletions src/core/src/main/java/org/apache/jmeter/testelement/AbstractTestElement.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -482,10 +482,16 @@ public void setProperty(JMeterProperty property) {
} else {
writeLock();
try {
propMap.put(property.getName(), property);
Map<String, JMeterProperty> propMapConcurrent = this.propMapConcurrent;
if (propMapConcurrent != null) {
propMapConcurrent.put(property.getName(), property);
if (property instanceof StringProperty && property.getStringValue() == null) {
// Avoid storing properties with null values since they will be skipped anyway when saving
// the test plan
removeProperty(property.getName());
} else {
propMap.put(property.getName(), property);
Map<String, JMeterProperty> propMapConcurrent = this.propMapConcurrent;
if (propMapConcurrent != null) {
propMapConcurrent.put(property.getName(), property);
}
}
} finally {
writeUnlock();
Expand Down
2 changes: 1 addition & 1 deletion src/core/src/main/kotlin/org/apache/jmeter/dsl/DslPrinterTraverser.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ public class DslPrinterTraverser(
is DoubleProperty -> append(property.stringValue)
is FloatProperty -> append(property.stringValue).append('f')
is LongProperty -> append(property.stringValue).append('d')
is StringProperty -> appendLiteral(property.stringValue)
is StringProperty -> property.stringValue?.let { appendLiteral(it) } ?: run { append("null") }

is TestElementProperty -> {
val element = property.element
Expand Down
78 changes: 78 additions & 0 deletions src/dist-check/src/test/java/org/apache/jmeter/junit/JMeterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,15 @@
import java.io.Serializable;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Modifier;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Objects;
import java.util.Properties;
import java.util.stream.Collectors;

Expand All @@ -45,13 +47,16 @@

import org.apache.commons.lang3.StringUtils;
import org.apache.jmeter.config.gui.ObsoleteGui;
import org.apache.jmeter.dsl.DslPrinterTraverser;
import org.apache.jmeter.gui.JMeterGUIComponent;
import org.apache.jmeter.gui.UnsharedComponent;
import org.apache.jmeter.gui.tree.JMeterTreeNode;
import org.apache.jmeter.save.SaveService;
import org.apache.jmeter.testbeans.TestBean;
import org.apache.jmeter.testbeans.gui.TestBeanGUI;
import org.apache.jmeter.testelement.TestElement;
import org.apache.jmeter.testelement.property.JMeterProperty;
import org.apache.jmeter.testelement.property.PropertyIterator;
import org.apache.jmeter.util.JMeterUtils;
import org.apache.jorphan.reflect.ClassFinder;
import org.apache.jorphan.util.JOrphanUtils;
Expand Down Expand Up @@ -270,6 +275,8 @@ private static Test suiteGUIComponents() throws Throwable {
System.out.println("o.a.j.junit.JMeterTest INFO: JMeterGUIComponent: skipping some tests " + item.getClass().getName());
} else {
ts.addTest(new JMeterTest("GUIComponents2", item));
ts.addTest(new JMeterTest("saveLoadShouldKeepElementIntact", item));
ts.addTest(new JMeterTest("propertiesShouldNotBeInitializedToNullValues", item));
ts.addTest(new JMeterTest("runGUITitle", item));
}
suite.addTest(ts);
Expand All @@ -289,6 +296,8 @@ private static Test suiteBeanComponents() throws Throwable {
JMeterGUIComponent item = new TestBeanGUI(c);
TestSuite ts = new TestSuite(item.getClass().getName());
ts.addTest(new JMeterTest("GUIComponents2", item));
ts.addTest(new JMeterTest("saveLoadShouldKeepElementIntact", item));
ts.addTest(new JMeterTest("propertiesShouldNotBeInitializedToNullValues", item));
ts.addTest(new JMeterTest("runGUITitle", item));
suite.addTest(ts);
} catch (IllegalArgumentException e) {
Expand Down Expand Up @@ -386,6 +395,75 @@ public void GUIComponents2() throws Exception {
assertEquals("Modify Test: Failed on " + name, "hey, new name!:", el2.getName());
}

public void propertiesShouldNotBeInitializedToNullValues() {
TestElement el = guiItem.createTestElement();
PropertyIterator it = el.propertyIterator();
while (it.hasNext()) {
JMeterProperty property = it.next();
if (property.getObjectValue() == null) {
fail(
"Property " + property.getName() + " is initialized with NULL OBJECT value in " +
" test element " + el + " created with " + guiItem + ".createTestElement() " +
"Please refrain from that since null properties consume memory, and they will be " +
"removed when saving and loading the plan anyway"
);
}
if (property.getStringValue() == null) {
fail(
"Property " + property.getName() + " is initialized with NULL STRING value in " +
" test element " + el + " created with " + guiItem + ".createTestElement() " +
"Please refrain from that since null properties consume memory, and they will be " +
"removed when saving and loading the plan anyway"
);
}
}
}

public void saveLoadShouldKeepElementIntact() throws IOException {
TestElement expected = guiItem.createTestElement();
ByteArrayOutputStream bos = new ByteArrayOutputStream();
SaveService.saveElement(expected, bos);
byte[] serializedBytes = bos.toByteArray();
TestElement actual = (TestElement) SaveService.loadElement(new ByteArrayInputStream(serializedBytes));
compareAllProperties(expected, actual, serializedBytes);
}

private static void compareAllProperties(TestElement expected, TestElement actual, byte[] serializedBytes) {
// JMeter restores "enabled" as StringProperty, see
// org.apache.jmeter.save.converters.ConversionHelp.restoreSpecialProperties
// So we normalize it back to BooleanProperty
JMeterProperty expEnabled = expected.getPropertyOrNull(expected.getSchema().getEnabled());
if (expEnabled != null && (expEnabled.getStringValue().equals("true") || expEnabled.getStringValue().equals("false"))) {
JMeterProperty actEnabled = actual.getPropertyOrNull(actual.getSchema().getEnabled());
if (actEnabled != null && actEnabled.getStringValue().equals(expEnabled.getStringValue())) {
actual.setProperty(expEnabled);
}
}

String expectedStr = new DslPrinterTraverser(DslPrinterTraverser.DetailLevel.ALL).append(expected).toString();
if (!Objects.equals(expected, actual)) {
boolean abc = Objects.equals(expected, actual);
assertEquals(
"TestElement after 'save+load' should match the one created in GUI\n" +
"JMX is " + new String(serializedBytes, StandardCharsets.UTF_8),
expectedStr,
new DslPrinterTraverser(DslPrinterTraverser.DetailLevel.ALL).append(actual).toString()
);
fail("TestElement after 'save+load' should match the one created in GUI. " +
"DSL representation is the same, however TestElement#equals says the elements are different. " +
"DSL is " + expectedStr + "\n" +
"JMX is " + new String(serializedBytes, StandardCharsets.UTF_8));
}
assertEquals(
"TestElement.hashCode after 'save+load' should match the one created in GUI. " +
"DSL representation is the same, however TestElement#hashCode says the elements are different. " +
"DSL is " + expectedStr + "\n" +
"JMX is " + new String(serializedBytes, StandardCharsets.UTF_8),
expected.hashCode(),
actual.hashCode()
);
}

/*
* Test serializable elements - create the suite of tests
*/
Expand Down
2 changes: 1 addition & 1 deletion src/dist-check/src/test/java/org/apache/jmeter/save/TestSaveService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ private boolean loadAndSave(File testFile, String fileName, boolean checkSize, F
final FileStats origStats = getFileStats(testFile);
final FileStats savedStats = getFileStats(savedFile);

ByteArrayOutputStream out = new ByteArrayOutputStream(1000000);
ByteArrayOutputStream out = new ByteArrayOutputStream(Math.toIntExact(testFile.length()));
try {
HashTree tree = SaveService.loadTree(testFile);
SaveService.saveTree(tree, out);
Expand Down
13 changes: 11 additions & 2 deletions src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/proxy/ProxyControl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import java.util.Set;
import java.util.prefs.Preferences;
import java.util.regex.PatternSyntaxException;
import java.util.stream.Collectors;

import org.apache.commons.codec.binary.Base64;
import org.apache.commons.codec.digest.DigestUtils;
Expand Down Expand Up @@ -398,11 +399,19 @@ public void setNotifyChildSamplerListenerOfFilteredSamplers(boolean b) {
}

public void setIncludeList(Collection<String> list) {
setProperty(new CollectionProperty(INCLUDE_LIST, new HashSet<>(list)));
if (list.size() >= 2) {
// Deduplicate if there is more than one element in the list
list = list.stream().distinct().collect(Collectors.toList());
}
setProperty(new CollectionProperty(INCLUDE_LIST, list));
}

public void setExcludeList(Collection<String> list) {
setProperty(new CollectionProperty(EXCLUDE_LIST, new HashSet<>(list)));
if (list.size() >= 2) {
// Deduplicate if there is more than one element in the list
list = list.stream().distinct().collect(Collectors.toList());
}
setProperty(new CollectionProperty(EXCLUDE_LIST, list));
}

public void setRegexMatch(boolean b) {
Expand Down

0 comments on commit 1e45414

Please sign in to comment.