Fix duplicate menu entries and inconsistent Test Plan naming during merge.

.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

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;

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()));

src/core/src/main/java/org/apache/jmeter/gui/tree/JMeterTreeModel.java

@@ -110,16 +110,41 @@ public JMeterTreeNode getNodeOf(TestElement userObject) {
      *             <code>subTree</code>
      */
     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
+     *            <code>current</code>
+     * @param current
+     *            The node in which the <code>subTree</code> 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 <code>current</code>
+     * @throws IllegalUserActionException
+     *             when <code>current</code> is not an instance of
+     *             {@link AbstractConfigGui} and no instance of {@link TestPlan}
+     *             <code>subTree</code>
+     */
+    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);

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));

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)}.
+ *
+ * <p>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.
+     *
+     * <p>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)");
+    }
+}

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.
+ *
+ * <p>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).
+ *
+ * <p>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.
+ *
+ * <p>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<String> 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<String> 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<String> 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<String> 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<String> 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<String> 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");
+    }
+}