From 8e609dc196d95414aff7f663c0464ca80163d3d3 Mon Sep 17 00:00:00 2001 From: ruthes00 Date: Mon, 21 Sep 2026 15:04:20 -0400 Subject: [PATCH 1/3] Duplicate menu entries and inconsistent Test Plan naming during merge in JMeter 5.6.3. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug 1: Inconsistent Test Plan Name After Merge. When the loaded file's root element is a `TestPlan`, `addSubTree` always resets the current plan's name to the incoming plan's name. Bug 2: Duplicate Menu Entries (Open, Merge, Save Selection As). `MenuFactory.addFileMenu()` is added to every node's popup, and that popup is mirrored into the Edit menu bar. Menu entries should be consistent and non‑duplicated. • File menu should contain file‑level actions. • Edit menu should contain node‑level actions. • Merge behavior should be predictable: either always preserve the current Test Plan name or always adopt the merged file’s name, regardless of selected node. Closes https://github.com/apache/jmeter/issues/6633 --- .../org/apache/jmeter/gui/GuiPackage.java | 20 ++++++++++++- .../org/apache/jmeter/gui/action/Load.java | 2 +- .../jmeter/gui/tree/JMeterTreeModel.java | 29 +++++++++++++++++-- .../apache/jmeter/gui/util/MenuFactory.java | 6 ++-- 4 files changed, 50 insertions(+), 7 deletions(-) diff --git a/src/core/src/main/java/org/apache/jmeter/gui/GuiPackage.java b/src/core/src/main/java/org/apache/jmeter/gui/GuiPackage.java index a08b0f5a9c6..32e6e3d0fa7 100644 --- a/src/core/src/main/java/org/apache/jmeter/gui/GuiPackage.java +++ b/src/core/src/main/java/org/apache/jmeter/gui/GuiPackage.java @@ -551,7 +551,25 @@ public boolean isDirty() { * if a subtree cannot be added to the currently selected node */ public HashTree addSubTree(HashTree subTree) throws IllegalUserActionException { - HashTree hashTree = treeModel.addSubTree(subTree, treeListener.getCurrentNode()); + return addSubTree(subTree, false); + } + + /** + * Add a subtree to the currently selected node. + * + * @param subTree + * the subtree to add. + * @param merging + * if {@code true}, the existing Test Plan name is preserved and + * not replaced by the name from the loaded file. + * + * @return the resulting subtree starting with the currently selected node + * + * @throws IllegalUserActionException + * if a subtree cannot be added to the currently selected node + */ + public HashTree addSubTree(HashTree subTree, boolean merging) throws IllegalUserActionException { + HashTree hashTree = treeModel.addSubTree(subTree, treeListener.getCurrentNode(), merging); undoHistory.clear(); undoHistory.add(this.treeModel, "Loaded tree"); return hashTree; diff --git a/src/core/src/main/java/org/apache/jmeter/gui/action/Load.java b/src/core/src/main/java/org/apache/jmeter/gui/action/Load.java index 10b997d141d..c13e569d6ae 100644 --- a/src/core/src/main/java/org/apache/jmeter/gui/action/Load.java +++ b/src/core/src/main/java/org/apache/jmeter/gui/action/Load.java @@ -198,7 +198,7 @@ public static boolean insertLoadedTree(final int id, final HashTree tree, final } } } - final HashTree newTree = guiInstance.addSubTree(tree); + final HashTree newTree = guiInstance.addSubTree(tree, merging); guiInstance.updateCurrentGui(); guiInstance.getMainFrame().getTree().setSelectionPath( new TreePath(((JMeterTreeNode) newTree.getArray()[0]).getPath())); diff --git a/src/core/src/main/java/org/apache/jmeter/gui/tree/JMeterTreeModel.java b/src/core/src/main/java/org/apache/jmeter/gui/tree/JMeterTreeModel.java index a2411135e77..3a803eccbf4 100644 --- a/src/core/src/main/java/org/apache/jmeter/gui/tree/JMeterTreeModel.java +++ b/src/core/src/main/java/org/apache/jmeter/gui/tree/JMeterTreeModel.java @@ -110,16 +110,41 @@ public JMeterTreeNode getNodeOf(TestElement userObject) { * subTree */ public HashTree addSubTree(HashTree subTree, JMeterTreeNode current) throws IllegalUserActionException { + return addSubTree(subTree, current, false); + } + + /** + * Adds the sub tree at the given node. Returns a boolean indicating whether + * the added sub tree was a full test plan. + * + * @param subTree + * The {@link HashTree} which is to be inserted into + * current + * @param current + * The node in which the subTree is to be inserted. + * Will be overridden, when an instance of {@link TestPlan} + * @param merging + * If {@code true}, the name of the existing {@link TestPlan} is + * preserved and not replaced by the name from the loaded file. + * @return newly created sub tree now found at current + * @throws IllegalUserActionException + * when current is not an instance of + * {@link AbstractConfigGui} and no instance of {@link TestPlan} + * subTree + */ + public HashTree addSubTree(HashTree subTree, JMeterTreeNode current, boolean merging) throws IllegalUserActionException { for (Object o : subTree.list()) { TestElement item = (TestElement) o; if (item instanceof TestPlan tp) { current = (JMeterTreeNode) ((JMeterTreeNode) getRoot()).getChildAt(0); final TestPlan userObject = (TestPlan) current.getUserObject(); userObject.addTestElement(item); - userObject.setName(item.getName()); + if (!merging) { + userObject.setName(item.getName()); + } userObject.setFunctionalMode(tp.isFunctionalMode()); userObject.setSerialized(tp.isSerialized()); - addSubTree(subTree.getTree(item), current); + addSubTree(subTree.getTree(item), current, merging); } else if (isWorkbench(item)) { //Move item from WorkBench to TestPlan HashTree workbenchTree = subTree.getTree(item); diff --git a/src/core/src/main/java/org/apache/jmeter/gui/util/MenuFactory.java b/src/core/src/main/java/org/apache/jmeter/gui/util/MenuFactory.java index 92d825fb89a..8f58ddca9c9 100644 --- a/src/core/src/main/java/org/apache/jmeter/gui/util/MenuFactory.java +++ b/src/core/src/main/java/org/apache/jmeter/gui/util/MenuFactory.java @@ -289,9 +289,9 @@ public static void addFileMenu(JPopupMenu menu, boolean addSaveTestFragmentMenu) } addSeparator(menu); - menu.add(makeMenuItemRes("open", ActionNames.OPEN));// $NON-NLS-1$ - menu.add(makeMenuItemRes("menu_merge", ActionNames.MERGE));// $NON-NLS-1$ - menu.add(makeMenuItemRes("save_as", ActionNames.SAVE_AS));// $NON-NLS-1$ + // Note: Open and Merge are intentionally omitted here; they are file-level + // actions that already appear in the File menu bar and must not be duplicated + // in the node right-click / Edit menu (see GitHub issue #6633). if(addSaveTestFragmentMenu) { menu.add(makeMenuItemRes("save_as_test_fragment", // $NON-NLS-1$ ActionNames.SAVE_AS_TEST_FRAGMENT)); From 2924f9947a3cc71c48a3e07b7d74dac2fa7f6147 Mon Sep 17 00:00:00 2001 From: ruthes00 Date: Mon, 21 Sep 2026 15:32:23 -0400 Subject: [PATCH 2/3] Duplicate menu entries and inconsistent Test Plan naming during merge in JMeter 5.6.3. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug 1: Inconsistent Test Plan Name After Merge. When the loaded file's root element is a `TestPlan`, `addSubTree` always resets the current plan's name to the incoming plan's name. Bug 2: Duplicate Menu Entries (Open, Merge, Save Selection As). `MenuFactory.addFileMenu()` is added to every node's popup, and that popup is mirrored into the Edit menu bar. Menu entries should be consistent and non‑duplicated. • File menu should contain file‑level actions. • Edit menu should contain node‑level actions. • Merge behavior should be predictable: either always preserve the current Test Plan name or always adopt the merged file’s name, regardless of selected node. Closes https://github.com/apache/jmeter/issues/6633 --- .../gui/tree/JMeterTreeModelMergeTest.java | 116 ++++++++++++++ .../gui/util/MenuFactoryFileMenuTest.java | 148 ++++++++++++++++++ 2 files changed, 264 insertions(+) create mode 100644 src/core/src/test/java/org/apache/jmeter/gui/tree/JMeterTreeModelMergeTest.java create mode 100644 src/dist-check/src/test/java/org/apache/jmeter/gui/util/MenuFactoryFileMenuTest.java diff --git a/src/core/src/test/java/org/apache/jmeter/gui/tree/JMeterTreeModelMergeTest.java b/src/core/src/test/java/org/apache/jmeter/gui/tree/JMeterTreeModelMergeTest.java new file mode 100644 index 00000000000..1ac1a38aa77 --- /dev/null +++ b/src/core/src/test/java/org/apache/jmeter/gui/tree/JMeterTreeModelMergeTest.java @@ -0,0 +1,116 @@ +/* + * 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.jmeter.gui.tree; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import org.apache.jmeter.testelement.TestPlan; +import org.apache.jorphan.collections.HashTree; +import org.junit.jupiter.api.Test; + +/** + * Tests for the merge behaviour of {@link JMeterTreeModel#addSubTree(HashTree, JMeterTreeNode, boolean)}. + * + *

Covers GitHub issue #6633: when {@code merging=true} the existing Test Plan name must be + * preserved; when {@code merging=false} (normal load) the name from the loaded file is adopted. + */ +class JMeterTreeModelMergeTest { + + // ----------------------------------------------------------------------- + // helpers + // ----------------------------------------------------------------------- + + /** Build a minimal HashTree whose root is a TestPlan with the given name. */ + private static HashTree buildPlanTree(String planName) { + TestPlan plan = new TestPlan(planName); + HashTree tree = new HashTree(); + tree.add(plan); + return tree; + } + + /** Return the TestPlan held at the first child of the model root. */ + private static TestPlan rootPlan(JMeterTreeModel model) { + JMeterTreeNode root = (JMeterTreeNode) model.getRoot(); + JMeterTreeNode planNode = (JMeterTreeNode) root.getChildAt(0); + return (TestPlan) planNode.getUserObject(); + } + + // ----------------------------------------------------------------------- + // tests + // ----------------------------------------------------------------------- + + /** + * When merging=true the existing Test Plan name must NOT be overwritten, + * regardless of the name carried by the incoming file. + * + *

This is the primary regression test for issue #6633 (inconsistent + * Test Plan naming after merge). + */ + @SuppressWarnings("deprecation") // JMeterTreeModel(Object) is the intended non-GUI constructor + @Test + void merging_preservesExistingTestPlanName() throws Exception { + // Arrange – model initialised with "Test Plan A" + JMeterTreeModel model = new JMeterTreeModel(new Object()); // non-GUI constructor + rootPlan(model).setName("Test Plan A"); + + // Act – merge a file whose root plan is named "Test Plan B" + JMeterTreeNode currentNode = (JMeterTreeNode) ((JMeterTreeNode) model.getRoot()).getChildAt(0); + model.addSubTree(buildPlanTree("Test Plan B"), currentNode, /* merging= */ true); + + // Assert – the model still shows "Test Plan A" + assertEquals("Test Plan A", rootPlan(model).getName(), + "Merge must not overwrite the existing Test Plan name"); + } + + /** + * When merging=false (normal open/load) the Test Plan name IS replaced by + * the name from the loaded file. + */ + @SuppressWarnings("deprecation") // JMeterTreeModel(Object) is the intended non-GUI constructor + @Test + void loading_replacesTestPlanName() throws Exception { + // Arrange + JMeterTreeModel model = new JMeterTreeModel(new Object()); + rootPlan(model).setName("Test Plan A"); + + // Act – load (not merge) a file whose root plan is named "Test Plan B" + JMeterTreeNode currentNode = (JMeterTreeNode) ((JMeterTreeNode) model.getRoot()).getChildAt(0); + model.addSubTree(buildPlanTree("Test Plan B"), currentNode, /* merging= */ false); + + // Assert – the model now shows "Test Plan B" + assertEquals("Test Plan B", rootPlan(model).getName(), + "Load (non-merge) must adopt the name from the loaded file"); + } + + /** + * The zero-argument convenience overload {@link JMeterTreeModel#addSubTree(HashTree, JMeterTreeNode)} + * must behave identically to calling the three-argument form with {@code merging=false}. + */ + @SuppressWarnings("deprecation") // JMeterTreeModel(Object) is the intended non-GUI constructor + @Test + void defaultOverload_behavesLikeNonMerge() throws Exception { + JMeterTreeModel model = new JMeterTreeModel(new Object()); + rootPlan(model).setName("Original"); + + JMeterTreeNode currentNode = (JMeterTreeNode) ((JMeterTreeNode) model.getRoot()).getChildAt(0); + model.addSubTree(buildPlanTree("Loaded"), currentNode); // two-arg form + + assertEquals("Loaded", rootPlan(model).getName(), + "Two-argument addSubTree must replace the plan name (non-merge semantics)"); + } +} diff --git a/src/dist-check/src/test/java/org/apache/jmeter/gui/util/MenuFactoryFileMenuTest.java b/src/dist-check/src/test/java/org/apache/jmeter/gui/util/MenuFactoryFileMenuTest.java new file mode 100644 index 00000000000..56760bb6a02 --- /dev/null +++ b/src/dist-check/src/test/java/org/apache/jmeter/gui/util/MenuFactoryFileMenuTest.java @@ -0,0 +1,148 @@ +/* + * 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.jmeter.gui.util; + +import static org.junit.jupiter.api.Assertions.assertFalse; + +import java.awt.Component; +import java.util.ArrayList; +import java.util.List; + +import javax.swing.JMenuItem; +import javax.swing.JPopupMenu; + +import org.apache.jmeter.gui.action.ActionNames; +import org.apache.jmeter.junit.JMeterTestCase; +import org.junit.jupiter.api.Test; + +/** + * Integration tests that verify {@link MenuFactory#addFileMenu(JPopupMenu)} does NOT add + * the file-level actions (Open, Merge, Save Selection As) to the node popup menu. + * + *

Those actions already live in the application's File menu bar and must not be + * duplicated in the right-click / Edit menu (GitHub issue #6633). + * + *

This test lives in the {@code dist-check} module because {@link MenuFactory}'s + * static initializer scans the JMeter classpath for GUI components, which requires + * the full JMeter distribution to be present on the classpath. + * + *

Because {@code addFileMenu} calls {@link org.apache.jmeter.gui.GuiPackage#getInstance()} + * later in its body (for image-save and enable/disable items), and {@code GuiPackage} is + * {@code null} in headless tests, we use a {@link RecordingPopupMenu} that captures every + * item added to it. The items we care about (Open, Merge, Save Selection As) are added – + * or intentionally omitted – before the {@code GuiPackage} call, so the assertion is valid. + */ +public class MenuFactoryFileMenuTest extends JMeterTestCase { + + /** + * A {@link JPopupMenu} subclass that records the action command of every + * {@link JMenuItem} added to it. + */ + private static final class RecordingPopupMenu extends JPopupMenu { + private static final long serialVersionUID = 1L; + final List actionCommands = new ArrayList<>(); + + @Override + public Component add(Component comp) { + if (comp instanceof JMenuItem item) { + String cmd = item.getActionCommand(); + if (cmd != null) { + actionCommands.add(cmd); + } + } + return super.add(comp); + } + } + + /** + * Collect all action commands that {@code addFileMenu} adds to the popup. + * The method will throw a {@link NullPointerException} when it tries to + * dereference {@code GuiPackage.getInstance()} (which is {@code null} in + * headless tests); we catch that and return whatever was recorded up to + * that point. The three actions we are testing are added before the NPE. + */ + private static List collectActionCommands(boolean addSaveTestFragment) { + RecordingPopupMenu popup = new RecordingPopupMenu(); + try { + MenuFactory.addFileMenu(popup, addSaveTestFragment); + } catch (NullPointerException ignored) { + // Expected: GuiPackage.getInstance() returns null in headless tests. + // Items added before that call are already recorded. + } + return popup.actionCommands; + } + + // ----------------------------------------------------------------------- + // tests + // ----------------------------------------------------------------------- + + /** + * {@code ActionNames.OPEN} must NOT appear in the node popup menu. + * It is a file-level action that belongs only in the File menu bar. + */ + @Test + public void addFileMenu_doesNotContainOpenAction() { + List commands = collectActionCommands(true); + assertFalse(commands.contains(ActionNames.OPEN), + "ActionNames.OPEN must not be added to the node popup / Edit menu " + + "(it duplicates the File menu entry – issue #6633). " + + "Found action commands: " + commands); + } + + /** + * {@code ActionNames.MERGE} must NOT appear in the node popup menu. + * It is a file-level action that belongs only in the File menu bar. + */ + @Test + public void addFileMenu_doesNotContainMergeAction() { + List commands = collectActionCommands(true); + assertFalse(commands.contains(ActionNames.MERGE), + "ActionNames.MERGE must not be added to the node popup / Edit menu " + + "(it duplicates the File menu entry – issue #6633). " + + "Found action commands: " + commands); + } + + /** + * {@code ActionNames.SAVE_AS} (Save Selection As) must NOT appear in the + * node popup menu. It is a file-level action that belongs only in the + * File menu bar. + */ + @Test + public void addFileMenu_doesNotContainSaveAsAction() { + List commands = collectActionCommands(true); + assertFalse(commands.contains(ActionNames.SAVE_AS), + "ActionNames.SAVE_AS must not be added to the node popup / Edit menu " + + "(it duplicates the File menu entry – issue #6633). " + + "Found action commands: " + commands); + } + + /** + * The same three actions must also be absent when the + * {@code addSaveTestFragmentMenu=false} variant is used. + */ + @Test + public void addFileMenu_noFragment_doesNotContainDuplicateFileActions() { + List commands = collectActionCommands(false); + assertFalse(commands.contains(ActionNames.OPEN), + "OPEN must not appear even when addSaveTestFragmentMenu=false"); + assertFalse(commands.contains(ActionNames.MERGE), + "MERGE must not appear even when addSaveTestFragmentMenu=false"); + assertFalse(commands.contains(ActionNames.SAVE_AS), + "SAVE_AS must not appear even when addSaveTestFragmentMenu=false"); + } +} From e8a579c8db65a0935509b79a89fe553355b25445 Mon Sep 17 00:00:00 2001 From: ruthes00 Date: Mon, 21 Sep 2026 15:53:35 -0400 Subject: [PATCH 3/3] Fixed build break by updating exact SHA hash and tag currently permitted for gradle/actions/wrapper-validation. --- .github/workflows/gradle-wrapper-validation.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/gradle-wrapper-validation.yml b/.github/workflows/gradle-wrapper-validation.yml index 78ee9d201e9..0910c38fd77 100644 --- a/.github/workflows/gradle-wrapper-validation.yml +++ b/.github/workflows/gradle-wrapper-validation.yml @@ -7,4 +7,4 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v6 - - uses: gradle/actions/wrapper-validation@0723195856401067f7a2779048b490ace7a47d7c # v5.0.2 + - uses: gradle/actions/wrapper-validation@3f131e8634966bd73d06cc69884922b02e6faf92 # v6.2.0