Changeset 18804 in josm for trunk/test/unit/org

Timestamp:

2023-08-13T00:20:18+02:00 (15 months ago)

Author:

taylor.smock

Message:

Fix #23116: Adding a member to a newly created relation will cause an NPE

This was caused by returning a null relation for a new relation.

File:

• trunk/test/unit/org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditorTest.java (modified) (7 diffs)

trunk/test/unit/org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditorTest.java

@@ -2 +2 @@
 package org.openstreetmap.josm.gui.dialogs.relation;
 
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertSame;
-
+import static org.openstreetmap.josm.tools.I18n.tr;
+
+import java.awt.Container;
+import java.awt.event.WindowEvent;
 import java.util.Collection;
 import java.util.Collections;
-
+import java.util.concurrent.atomic.AtomicReference;
+
+import javax.swing.Action;
+import javax.swing.JButton;
 import javax.swing.JOptionPane;
 import javax.swing.JPanel;
 
+import mockit.Invocation;
 import mockit.Mock;
 import mockit.MockUp;
+
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.platform.commons.support.ReflectionSupport;
@@ -22 +34 @@
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
 import org.openstreetmap.josm.data.osm.Relation;
+import org.openstreetmap.josm.data.osm.RelationMember;
 import org.openstreetmap.josm.data.osm.Way;
+import org.openstreetmap.josm.gui.MainApplication;
+import org.openstreetmap.josm.gui.SideButton;
+import org.openstreetmap.josm.gui.dialogs.RelationListDialog;
+import org.openstreetmap.josm.gui.dialogs.relation.actions.AddSelectedAtStartAction;
 import org.openstreetmap.josm.gui.dialogs.relation.actions.IRelationEditorActionAccess;
 import org.openstreetmap.josm.gui.dialogs.relation.actions.PasteMembersAction;
@@ -31 +48 @@
 import org.openstreetmap.josm.testutils.annotations.BasicPreferences;
 import org.openstreetmap.josm.testutils.annotations.Main;
+import org.openstreetmap.josm.testutils.annotations.Projection;
 import org.openstreetmap.josm.testutils.mockers.JOptionPaneSimpleMocker;
 import org.openstreetmap.josm.testutils.mockers.WindowMocker;
@@ -39 +57 @@
 @BasicPreferences
 @Main
+@Projection
 public class GenericRelationEditorTest {
     /**
@@ -80 +99 @@
             }
         };
+    }
+
+    @BeforeEach
+    void setup() {
+        new PasteMembersActionMock();
+        new WindowMocker();
     }
 
@@ -137 +162 @@
     @Test
     void testNonRegression23091() throws Exception {
-        new MockUp<PasteMembersAction>() {
-            @Mock
-            protected void updateEnabledState() {
-                // Do nothing
-            }
-        };
-        new WindowMocker();
-
         DataSet ds = new DataSet();
         Relation relation = new Relation(1);
@@ -161 +178 @@
         assertSame(relation, selection.iterator().next(), "The selection should be the same");
     }
+
+    /**
+     * Ensure that users can create new relations and modify them.
+     */
+    @Test
+    void testNonRegression23116() {
+        // Setup the mocks
+        final AtomicReference<RelationEditor> editorReference = new AtomicReference<>();
+        new MockUp<RelationEditor>() {
+            @Mock public RelationEditor getEditor(Invocation invocation, OsmDataLayer layer, Relation r,
+                    Collection<RelationMember> selectedMembers) {
+                editorReference.set(invocation.proceed(layer, r, selectedMembers));
+                return editorReference.get();
+            }
+        };
+        // We want to go through the `setVisible` code, just in case. So we have to mock the window location
+        new MockUp<GenericRelationEditor>() {
+            @Mock public void setVisible(boolean visible) {
+                // Do nothing. Ideally, we would just mock the awt methods called, but that would take a lot of mocking.
+            }
+        };
+        // Set up the data
+        final DataSet dataSet = new DataSet();
+        MainApplication.getLayerManager().addLayer(new OsmDataLayer(dataSet, "GenericRelationEditorTest.testNonRegression23116", null));
+        dataSet.addPrimitive(TestUtils.newNode(""));
+        dataSet.setSelected(dataSet.allPrimitives());
+        final RelationListDialog relationListDialog = new RelationListDialog();
+        try {
+            final Action newAction = ((SideButton) getComponent(relationListDialog, 2, 0, 0)).getAction();
+            assertEquals("class org.openstreetmap.josm.gui.dialogs.RelationListDialog$NewAction",
+                    newAction.getClass().toString());
+            // Now get the buttons we want to push
+            newAction.actionPerformed(null);
+            final GenericRelationEditor editor = assertInstanceOf(GenericRelationEditor.class, editorReference.get());
+            final JButton okAction = getComponent(editor, 0, 1, 0, 2, 0);
+            assertEquals(tr("Delete"), okAction.getText(), "OK is Delete until the relation actually has data");
+            assertNotNull(editor);
+            final TagEditorPanel tagEditorPanel = getComponent(editor, 0, 1, 0, 1, 0, 0, 1, 1);
+            // We need at least one tag for the action to not be "Delete".
+            tagEditorPanel.getModel().add("type", "someUnknownTypeHere");
+            final Action addAtStartAction = assertInstanceOf(AddSelectedAtStartAction.class,
+                    ((JButton) getComponent(editor, 0, 1, 0, 1, 0, 0, 2, 0, 2, 1, 2, 0, 0)).getAction());
+            // Perform the actual test.
+            assertDoesNotThrow(() -> addAtStartAction.actionPerformed(null));
+            assertDoesNotThrow(() -> okAction.getAction().actionPerformed(null));
+            assertFalse(dataSet.getRelations().isEmpty());
+            assertSame(dataSet.getNodes().iterator().next(),
+                    dataSet.getRelations().iterator().next().getMember(0).getNode());
+        } finally {
+            // This avoids an issue with the cleanup code and the mocks for this test
+            if (editorReference.get() != null) {
+                RelationDialogManager.getRelationDialogManager().windowClosed(new WindowEvent(editorReference.get(), 0));
+            }
+        }
+    }
+
+    @SuppressWarnings("unchecked")
+    private static <T extends Container> T getComponent(Container parent, int... tree) {
+        Container current = parent;
+        for (int i : tree) {
+            current = (Container) current.getComponent(i);
+        }
+        return (T) current;
+    }
+
+    private static class PasteMembersActionMock extends MockUp<PasteMembersAction> {
+        @Mock
+        protected void updateEnabledState() {
+            // Do nothing
+        }
+    }
 }