Skip to content

Commit

Permalink
test: verify that GuiComponent.configure+modifyTestElement does not m…
Browse files Browse the repository at this point in the history 
…odify element after creation

It helps to verify both createTestElement, configure, and modifyTestElement implementations,
so JMeter does not accidentally show treat test plan as dirty when the user
merely switches across elements

At the same time, it ensures test elements do not accidentally lose properties
when processing modifyTestElement
  • Loading branch information
@vlsi
vlsi committed Dec 25, 2023
1 parent 9b92985 commit c2de205
Show file tree
Hide file tree
Showing 11 changed files with 114 additions and 43 deletions.
47 changes: 20 additions & 27 deletions src/core/src/main/java/org/apache/jmeter/control/gui/TransactionControllerGui.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,11 @@

package org.apache.jmeter.control.gui;

import javax.swing.JCheckBox;

import org.apache.jmeter.control.TransactionController;
import org.apache.jmeter.control.TransactionControllerSchema;
import org.apache.jmeter.gui.GUIMenuSortOrder;
import org.apache.jmeter.gui.JBooleanPropertyEditor;
import org.apache.jmeter.gui.TestElementMetadata;
import org.apache.jmeter.gui.util.CheckBoxPanel;
import org.apache.jmeter.testelement.TestElement;
import org.apache.jmeter.util.JMeterUtils;
import org.apache.jorphan.gui.layout.VerticalLayout;
Expand All @@ -37,40 +36,36 @@ public class TransactionControllerGui extends AbstractControllerGui {
private static final long serialVersionUID = 240L;

/** If selected, then generate parent sample, otherwise as per original controller */
private JCheckBox generateParentSample;
private final JBooleanPropertyEditor generateParentSample =
new JBooleanPropertyEditor(
TransactionControllerSchema.INSTANCE.getGenearteParentSample(),
JMeterUtils.getResString("transaction_controller_parent"));

/** if selected, add duration of timers to total runtime */
private JCheckBox includeTimers;
private final JBooleanPropertyEditor includeTimers =
new JBooleanPropertyEditor(
TransactionControllerSchema.INSTANCE.getIncludeTimers(),
JMeterUtils.getResString("transaction_controller_include_timers"));

/**
* Create a new TransactionControllerGui instance.
*/
public TransactionControllerGui() {
init();
bindingGroup.add(generateParentSample);
bindingGroup.add(includeTimers);
}

@Override
public TestElement createTestElement() {
TransactionController lc = new TransactionController();
lc.setIncludeTimers(false); // change default for new test elements
configureTestElement(lc);
return lc;
}

@Override
public void configure(TestElement el) {
super.configure(el);
generateParentSample.setSelected(((TransactionController) el).isGenerateParentSample());
includeTimers.setSelected(((TransactionController) el).isIncludeTimers());
public TestElement makeTestElement() {
return new TransactionController();
}

@Override
public void modifyTestElement(TestElement el) {
configureTestElement(el);
((TransactionController) el).setGenerateParentSample(generateParentSample.isSelected());
TransactionController tc = (TransactionController) el;
tc.setGenerateParentSample(generateParentSample.isSelected());
tc.setIncludeTimers(includeTimers.isSelected());
public void assignDefaultValues(TestElement element) {
super.assignDefaultValues(element);
// See https://github.com/apache/jmeter/issues/3282
((TransactionController) element).setIncludeTimers(false);
}

@Override
Expand All @@ -85,9 +80,7 @@ private void init() { // WARNING: called from ctor so must not be overridden (i.
setLayout(new VerticalLayout(5, VerticalLayout.BOTH, VerticalLayout.TOP));
setBorder(makeBorder());
add(makeTitlePanel());
generateParentSample = new JCheckBox(JMeterUtils.getResString("transaction_controller_parent")); // $NON-NLS-1$
add(CheckBoxPanel.wrap(generateParentSample));
includeTimers = new JCheckBox(JMeterUtils.getResString("transaction_controller_include_timers"), true); // $NON-NLS-1$
add(CheckBoxPanel.wrap(includeTimers));
add(generateParentSample);
add(includeTimers);
}
}
6 changes: 6 additions & 0 deletions src/core/src/main/kotlin/org/apache/jmeter/threads/openmodel/gui/OpenModelThreadGroupGui.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import org.apache.jmeter.gui.TestElementMetadata
import org.apache.jmeter.testelement.TestElement
import org.apache.jmeter.threads.gui.AbstractThreadGroupGui
import org.apache.jmeter.threads.openmodel.OpenModelThreadGroup
import org.apache.jmeter.threads.openmodel.OpenModelThreadGroupController
import org.apache.jmeter.threads.openmodel.OpenModelThreadGroupSchema
import org.apache.jmeter.threads.openmodel.ThreadSchedule
import org.apache.jmeter.threads.openmodel.ThreadScheduleStep
Expand Down Expand Up @@ -140,4 +141,9 @@ public class OpenModelThreadGroupGui : AbstractThreadGroupGui() {
private fun evaluate(input: String): String = CompoundVariable(input).execute()

override fun makeTestElement(): TestElement = OpenModelThreadGroup()

override fun modifyTestElement(element: TestElement) {
super.modifyTestElement(element)
element[OpenModelThreadGroupSchema.mainController] = OpenModelThreadGroupController()
}
}
39 changes: 38 additions & 1 deletion 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 @@ -52,6 +52,8 @@
import org.apache.jmeter.gui.UnsharedComponent;
import org.apache.jmeter.gui.tree.JMeterTreeNode;
import org.apache.jmeter.loadsave.IsEnabledNormalizer;
import org.apache.jmeter.protocol.http.control.gui.GraphQLHTTPSamplerGui;
import org.apache.jmeter.protocol.http.sampler.HTTPSamplerBaseSchema;
import org.apache.jmeter.save.SaveService;
import org.apache.jmeter.testbeans.TestBean;
import org.apache.jmeter.testbeans.gui.TestBeanGUI;
Expand Down Expand Up @@ -278,6 +280,7 @@ private static Test suiteGUIComponents() throws Throwable {
ts.addTest(new JMeterTest("GUIComponents2", item));
ts.addTest(new JMeterTest("saveLoadShouldKeepElementIntact", item));
ts.addTest(new JMeterTest("propertiesShouldNotBeInitializedToNullValues", item));
ts.addTest(new JMeterTest("elementShouldNotBeModifiedWithConfigureModify", item));
ts.addTest(new JMeterTest("runGUITitle", item));
}
suite.addTest(ts);
Expand All @@ -299,6 +302,7 @@ private static Test suiteBeanComponents() throws Throwable {
ts.addTest(new JMeterTest("GUIComponents2", item));
ts.addTest(new JMeterTest("saveLoadShouldKeepElementIntact", item));
ts.addTest(new JMeterTest("propertiesShouldNotBeInitializedToNullValues", item));
ts.addTest(new JMeterTest("elementShouldNotBeModifiedWithConfigureModify", item));
ts.addTest(new JMeterTest("runGUITitle", item));
suite.addTest(ts);
} catch (IllegalArgumentException e) {
Expand Down Expand Up @@ -420,6 +424,39 @@ public void propertiesShouldNotBeInitializedToNullValues() {
}
}

public void elementShouldNotBeModifiedWithConfigureModify() {
TestElement expected = guiItem.createTestElement();
TestElement actual = guiItem.createTestElement();
guiItem.configure(actual);
if (!Objects.equals(expected, actual)) {
boolean breakpointForDebugging = Objects.equals(expected, actual);
String expectedStr = new DslPrinterTraverser(DslPrinterTraverser.DetailLevel.ALL).append(expected).toString();
String actualStr = new DslPrinterTraverser(DslPrinterTraverser.DetailLevel.ALL).append(actual).toString();
assertEquals(
"TestElement should not be modified by " + guiItem.getClass().getName() + ".configure(element)",
expectedStr,
actualStr
);
}
guiItem.modifyTestElement(actual);
if (guiItem.getClass() == GraphQLHTTPSamplerGui.class) {
// GraphQL sampler computes its arguments, so we don't compare them
// See org.apache.jmeter.protocol.http.config.gui.GraphQLUrlConfigGui.modifyTestElement
expected.removeProperty(HTTPSamplerBaseSchema.INSTANCE.getArguments());
actual.removeProperty(HTTPSamplerBaseSchema.INSTANCE.getArguments());
}
if (!Objects.equals(expected, actual)) {
boolean breakpointForDebugging = Objects.equals(expected, actual);
String expectedStr = new DslPrinterTraverser(DslPrinterTraverser.DetailLevel.ALL).append(expected).toString();
String actualStr = new DslPrinterTraverser(DslPrinterTraverser.DetailLevel.ALL).append(actual).toString();
assertEquals(
"TestElement should not be modified by " + guiItem.getClass().getName() + ".configure(element); gui.modifyTestElement(element)",
expectedStr,
actualStr
);
}
}

public void saveLoadShouldKeepElementIntact() throws IOException {
TestElement expected = guiItem.createTestElement();
ByteArrayOutputStream bos = new ByteArrayOutputStream();
Expand All @@ -435,7 +472,7 @@ private static void compareAllProperties(TestElement expected, TestElement actua

String expectedStr = new DslPrinterTraverser(DslPrinterTraverser.DetailLevel.ALL).append(expected).toString();
if (!Objects.equals(expected, actual)) {
boolean abc = Objects.equals(expected, actual);
boolean breakpointForDebugging = 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),
Expand Down
8 changes: 5 additions & 3 deletions ...ol/http/src/main/java/org/apache/jmeter/protocol/http/config/gui/GraphQLUrlConfigGui.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@

package org.apache.jmeter.protocol.http.config.gui;

import static org.apache.commons.lang3.StringUtils.defaultIfEmpty;

import java.awt.Component;

import javax.swing.BorderFactory;
Expand Down Expand Up @@ -93,9 +95,9 @@ public void modifyTestElement(TestElement element) {
final GraphQLRequestParams params = new GraphQLRequestParams(operationNameText.getText(),
queryContent.getText(), variablesContent.getText());

element.setProperty(OPERATION_NAME, params.getOperationName());
element.setProperty(QUERY, params.getQuery());
element.setProperty(VARIABLES, params.getVariables());
element.setProperty(OPERATION_NAME, defaultIfEmpty(params.getOperationName(), null));
element.setProperty(QUERY, defaultIfEmpty(params.getQuery(), null));
element.setProperty(VARIABLES, defaultIfEmpty(params.getVariables(), null));
element.setProperty(HTTPSamplerBase.POST_BODY_RAW, !HTTPConstants.GET.equals(method));

final Arguments args;
Expand Down
4 changes: 3 additions & 1 deletion src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/config/gui/UrlConfigGui.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,9 @@ public void modifyTestElement(TestElement element) {
}

public void assignDefaultValues(TestElement element) {
((HTTPSamplerBase) element).setArguments(argsPanel.createTestElement());
HTTPSamplerBase httpSampler = (HTTPSamplerBase) element;
httpSampler.setPostBodyRaw(false);
httpSampler.setArguments(argsPanel.createTestElement());
}

// Just append all the parameter values, and use that as the post body
Expand Down
16 changes: 14 additions & 2 deletions ...http/src/main/java/org/apache/jmeter/protocol/http/control/gui/GraphQLHTTPSamplerGui.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,20 @@ public void assignDefaultValues(TestElement element) {
super.assignDefaultValues(element);
HTTPSamplerBaseSchema schema = HTTPSamplerBaseSchema.INSTANCE;
element.set(schema.getMethod(), HTTPConstants.POST);
element.set(schema.getUseBrowserCompatibleMultipart(), false);
element.set(schema.getUseMultipartPost(), false);
element.set(schema.getPostBodyRaw(), true);
disableMultipart(element);
}

@Override
public void modifyTestElement(TestElement sampler) {
super.modifyTestElement(sampler);
disableMultipart(sampler);
}

private static void disableMultipart(TestElement sampler) {
HTTPSamplerBaseSchema schema = HTTPSamplerBaseSchema.INSTANCE;
sampler.set(schema.getUseBrowserCompatibleMultipart(), false);
sampler.set(schema.getUseMultipartPost(), false);
}

public GraphQLHTTPSamplerGui() {
Expand Down
3 changes: 2 additions & 1 deletion ...col/http/src/main/java/org/apache/jmeter/protocol/http/control/gui/HttpTestSampleGui.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,8 @@ public void modifyTestElement(TestElement sampler) {
} else {
samplerBase.removeProperty(httpSchema.getIpSourceType());
}
samplerBase.set(httpSchema.getImplementation(), String.valueOf(httpImplementation.getSelectedItem()));
String selectedImplementation = String.valueOf(httpImplementation.getSelectedItem());
samplerBase.set(httpSchema.getImplementation(), StringUtils.defaultIfBlank(selectedImplementation, null));
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/gui/AuthPanel.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,8 @@ public AuthPanel() {
public TestElement createTestElement() {
AuthManager authMan = tableModel.manager;
configureTestElement(authMan);
authMan.setClearEachIteration(clearEachIteration.isSelected());
authMan.setClearEachIteration(false);
authMan.setControlledByThread(false);
return (TestElement) authMan.clone();
}

Expand Down
21 changes: 17 additions & 4 deletions src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/gui/DNSCachePanel.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,10 +140,14 @@ public void modifyTestElement(TestElement dnsRes) {
configureTestElement(dnsRes);
if (dnsRes instanceof DNSCacheManager) {
DNSCacheManager dnsCacheManager = (DNSCacheManager) dnsRes;
// Init servers list
dnsCacheManager.getServers();
for (int i = 0; i < dnsServersTableModel.getRowCount(); i++) {
String server = (String) dnsServersTableModel.getRowData(i)[0];
dnsCacheManager.addServer(server);
}
// Init hosts list
dnsCacheManager.getHosts();
for (int i = 0; i < dnsHostsTableModel.getRowCount(); i++) {
String host = (String) dnsHostsTableModel.getRowData(i)[0];
String addresses = (String) dnsHostsTableModel.getRowData(i)[1];
Expand Down Expand Up @@ -188,10 +192,19 @@ private void populateHostsTable(DNSCacheManager resolver) {
}

@Override
public TestElement createTestElement() {
DNSCacheManager dnsCacheManager = new DNSCacheManager();
modifyTestElement(dnsCacheManager);
return dnsCacheManager;
public TestElement makeTestElement() {
return new DNSCacheManager();
}

@Override
public void assignDefaultValues(TestElement element) {
super.assignDefaultValues(element);
DNSCacheManager manager = (DNSCacheManager) element;
// It sets empty list of servers and hosts
manager.getServers();
manager.getHosts();
manager.setClearEachIteration(true);
manager.setCustomResolver(false);
}

@Override
Expand Down
8 changes: 5 additions & 3 deletions ...tocol/http/src/main/java/org/apache/jmeter/protocol/http/proxy/DefaultSamplerCreator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@

package org.apache.jmeter.protocol.http.proxy;

import static org.apache.commons.lang3.StringUtils.defaultIfEmpty;

import java.io.File;
import java.io.IOException;
import java.io.StringReader;
Expand Down Expand Up @@ -174,9 +176,9 @@ private static void detectAndModifySamplerOnGraphQLRequest(final HTTPSamplerBase

if (params != null) {
sampler.setProperty(TestElement.GUI_CLASS, GraphQLHTTPSamplerGui.class.getName());
sampler.setProperty(GraphQLUrlConfigGui.OPERATION_NAME, params.getOperationName());
sampler.setProperty(GraphQLUrlConfigGui.QUERY, params.getQuery());
sampler.setProperty(GraphQLUrlConfigGui.VARIABLES, params.getVariables());
sampler.setProperty(GraphQLUrlConfigGui.OPERATION_NAME, defaultIfEmpty(params.getOperationName(), null));
sampler.setProperty(GraphQLUrlConfigGui.QUERY, defaultIfEmpty(params.getQuery(), null));
sampler.setProperty(GraphQLUrlConfigGui.VARIABLES, defaultIfEmpty(params.getVariables(), null));
}
}

Expand Down
2 changes: 2 additions & 0 deletions ...l/http/src/test/kotlin/org/apache/jmeter/protocol/http/sampler/HttpSamplerPrintDslTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ class HttpSamplerPrintDslTest : JMeterTestCase() {
it[method] = "GET"
it[followRedirects] = true
it[useKeepalive] = true
it[postBodyRaw] = false
}
}
Expand Down Expand Up @@ -139,6 +140,7 @@ class HttpSamplerPrintDslTest : JMeterTestCase() {
it[method] = "GET"
it[followRedirects] = true
it[useKeepalive] = true
it[postBodyRaw] = false
}
}
}.keys.first() as TestElement
Expand Down

0 comments on commit c2de205

Please sign in to comment.