Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Enable usage of ${...} expressions for checkbox controls #5944

Merged
merged 7 commits into from Jun 6, 2023

Conversation

vlsi
Copy link
Collaborator

@vlsi vlsi commented May 26, 2023

Description

The checkbox can be converted to an editable field by calling a context menu:

  • right-click on the checkbox
  • or press shift+F10

The editable box converts back to a checkbox if you select "true" or "false"

Fixes #1252
See #5761

Motivation and Context

Previously, JMeter did not allow users to use ${...} expressions for checkbox elements, however, there are cases when the checkbox should be customizable.
For instance, users might want to test different values enable keepalive setting, and currently it is not possible to configure it from a property file.

How Has This Been Tested?

Only manual testing for now :-/

Screenshots (if appropriate):

The second Run Thread Groups consecutively element is the new checkbox. It behaves like a usual checkbox.

sample use of the new checkbox element

However, it has a popup menu (right-click or shift+F10) so the users can convert it to an expression:

popup menu for the new checkbox

The editable element looks like an editable combobox:

editable combo box instead of checkbox

Users can type up, down to select a predefined value, or they can select true or false in which case the element would collapse to a checkbox again:

combo box with available items

Open questions

It is unclear what we do with methods like org.apache.jmeter.testelement.TestPlan#setSerialized(boolean) and boolean org.apache.jmeter.testelement.TestPlan#isSerialized().

We might add a second pair of getter and setter for exposing the property as String, so the UI element can propagate the free-form text expression to the test plan configuration.

I remember I had the same issue with OpenModelThreadGroup where I created two properties randomSeed: Long and randomSeedString: String:

public val randomSeed: Long get() = getPropertyAsLong(RANDOM_SEED)
/**
* Random seed for building reproducible schedules. 0 means random seed.
*/
public var randomSeedString: String
get() = getPropertyAsString(RANDOM_SEED)
set(value) {
setProperty(RANDOM_SEED, value)
}

Duplicating properties does not look nice :-(

Checklist:

  • My code follows the code style of this project.
  • I have updated the documentation accordingly.

@vdaburon
Copy link
Contributor

Yes interresting feature. Be carreful with compatibility with current Jmeter script.

@vlsi
Copy link
Collaborator Author

vlsi commented May 29, 2023

I've added property descriptors, and the missing bit is to link "editable combobox" with BooleanPropertyDescriptor somehow

@vlsi
Copy link
Collaborator Author

vlsi commented May 30, 2023

I got TestPlan working with editable checkboxes.

Editable checkboxes in TestPlan UI

Output xml:

<?xml version="1.0" encoding="UTF-8"?>
<jmeterTestPlan version="1.2" properties="5.0" jmeter="5.5.1-SNAPSHOT b17bbbe">
  <hashTree>
    <TestPlan guiclass="TestPlanGui" testclass="TestPlan" testname="Test Plan" enabled="true">
      <stringProp name="TestPlan.comments"></stringProp>
      <stringProp name="TestPlan.functional_mode">asdfasdf</stringProp>
      <stringProp name="TestPlan.serialize_threadgroups">${__P(property_name)}</stringProp>
      <elementProp name="TestPlan.user_defined_variables" elementType="Arguments" guiclass="ArgumentsPanel" testclass="Arguments" testname="User Defined Variables" enabled="true">
        <collectionProp name="Arguments.arguments"/>
      </elementProp>
      <stringProp name="TestPlan.user_define_classpath"></stringProp>
      <stringProp name="TestPlan.tearDown_on_shutdown">${variable_name}</stringProp>
    </TestPlan>
    <hashTree/>
  </hashTree>
</jmeterTestPlan>

@vlsi vlsi force-pushed the editable_checkbox branch 2 times, most recently from 71951f9 to 3943b08 Compare May 31, 2023 11:18
@vlsi
Copy link
Collaborator Author

vlsi commented May 31, 2023

I rearranged the commits, and will split them into different PRs soon.

For reference, here are the changes to migrate TestPlan to editable checkboxes: 3943b08

-    private final JCheckBox serializedMode;
+    private final JBooleanPropertyEditor serializedMode =
+            new JBooleanPropertyEditor(TestPlanSchema.INSTANCE.getSerializeThreadgroups(), "testplan.serialized");

-            tp.setSerialized(serializedMode.isSelected());
+            serializedMode.updateElement(tp);

-            serializedMode.setSelected(tp.isSerialized());
+            serializedMode.updateUi(tp);

-        serializedMode.setSelected(false);
+        serializedMode.reset();

It looks good to me.

@vlsi vlsi force-pushed the editable_checkbox branch 4 times, most recently from abef36a to 3ad3446 Compare June 1, 2023 20:15
@vlsi vlsi force-pushed the editable_checkbox branch 6 times, most recently from bad1471 to 5a19be5 Compare June 3, 2023 19:13
Comment on lines +346 to 347
JMeterProperty jmp = getPropertyOrNull(key);
return jmp == null || jmp instanceof NullProperty ? defaultValue : jmp.getIntValue();
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Frankly speaking, getPropertyOrNull followed by instanceof NullProperty look strange, however, I do not change it in the current PR.
However, it would probably make sense to revise it later.

@vlsi vlsi force-pushed the editable_checkbox branch 5 times, most recently from 65cb835 to f9f292e Compare June 5, 2023 12:42
@vlsi
Copy link
Collaborator Author

vlsi commented Jun 5, 2023

I'm happy with the code and behaviour, so I will merge this soon

vlsi added 7 commits June 5, 2023 21:27
…rmer

The existing ValueTransformer includes "master function" and "variables"
that make no sense for most transformers.

The old interface is kept for backward compatibility
The checkbox can be converted to an editable field by calling a context menu:
* right-click on the checkbox
* or press shift+F10

The editable box converts back to a checkbox if you select "true" or "false"

Fixes apache#1252
See apache#5761
…ve, use_multipart_post, and browser_compatible_multipart

fixes apache#1252
@vlsi vlsi merged commit 5bb5669 into apache:master Jun 6, 2023
6 of 7 checks passed
@milamberspace
Copy link
Contributor

Perhaps need add refresh the UI when it's switch the checkbox to expression field (and return). See video.

simplescreenrecorder-2023-06-07_09.39.21.mp4

@vlsi
Copy link
Collaborator Author

vlsi commented Jun 7, 2023

Good catch. It updates automatically in Darklaf themes, and almost all the others fail to update.

@vlsi
Copy link
Collaborator Author

vlsi commented Jun 7, 2023

I've added relayout in f4d312c

vlsi added a commit that referenced this pull request Jun 7, 2023
…nges visible element

By default, CardLayout consumes the space which accomodates all its children,
however CardLayoutWithSizeOfCurrentVisibleElement consumes only the space
needed for the currently visible element.

That is why CardLayout does not need to trigger layout on changes.

Fixes #5944 (comment)
@vdaburon
Copy link
Contributor

Hi,
I think this new features (right click : context menu and select box) are not described in the Test Plan help documentation
https://jmeter.apache.org/usermanual/component_reference.html#Test_Plan
and
Http Sampler documentation
https://jmeter.apache.org/usermanual/component_reference.html#HTTP_Request

Regards.

@vlsi vlsi mentioned this pull request Mar 25, 2024
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

There is no good way to set checkbox based items using global variable.
3 participants