Changeset 18649 in josm

Timestamp:

2023-02-06T19:39:32+01:00 (22 months ago)

Author:

taylor.smock

Message:

Fix #22712: ignore list doesn't work

This occurred due to OsmValidator#save modifying the list, and expecting
all error codes to be positive.

TestErrorTest now checks one of the tests known to have a negative hashcode.

This also adds some additional JUnit 5 annotations. The additional
annotations are:

• @LayerManager (cleans up layers after each test run)
  • This is automatically registered for all tests
• @Projection (sets up the ProjectionRegistry for each test)

Location:

trunk

Files:

• src/org/openstreetmap/josm/data/validation/OsmValidator.java (modified) (2 diffs)
• test/data/META-INF (added)
• test/data/META-INF/services (added)
• test/data/META-INF/services/org.junit.jupiter.api.extension.Extension (added)
• test/unit/org/openstreetmap/josm/data/validation/TestErrorTest.java (modified) (5 diffs)
• test/unit/org/openstreetmap/josm/testutils/annotations/LayerManager.java (added)
• test/unit/org/openstreetmap/josm/testutils/annotations/Projection.java (added)

trunk/src/org/openstreetmap/josm/data/validation/OsmValidator.java

@@ -467 +467 @@
                 if (tr("Ignore list").equals(description))
                     description = "";
-                if (!key.matches("^[0-9]+(_.*|$)")) {
+                if (!key.matches("^-?[0-9]+(_.*|$)")) {
                     description = key;
                     key = "";
@@ -480 +480 @@
                     // single element
                     entry = key + ":" + item;
-                } else if (item.matches("^[0-9]+(_.*|)$")) {
+                } else if (item.matches("^-?[0-9]+(_.*|)$")) {
                     // no element ids
                     entry = item;

trunk/test/unit/org/openstreetmap/josm/data/validation/TestErrorTest.java

@@ -18 +18 @@
 import org.junit.jupiter.params.provider.MethodSource;
 import org.openstreetmap.josm.TestUtils;
+import org.openstreetmap.josm.data.coor.LatLon;
+import org.openstreetmap.josm.data.osm.DataSet;
+import org.openstreetmap.josm.data.osm.Node;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
 import org.openstreetmap.josm.data.validation.tests.InternetTags;
+import org.openstreetmap.josm.data.validation.tests.UnconnectedWays;
+import org.openstreetmap.josm.gui.MainApplication;
+import org.openstreetmap.josm.gui.layer.OsmDataLayer;
 import org.openstreetmap.josm.gui.progress.NullProgressMonitor;
 import org.openstreetmap.josm.testutils.annotations.BasicPreferences;
+import org.openstreetmap.josm.testutils.annotations.Projection;
 
 /**
@@ -27 +34 @@
  */
 @BasicPreferences
+@Projection
 class TestErrorTest {
     static Stream<Arguments> testCodeCompatibility() {
         return Stream.of(Arguments.of(InternetTags.class, 3301, 1166507262, false, Collections.singletonList(TestUtils.newNode("url=invalid"))),
-                Arguments.of(InternetTags.class, 3301, 1166507262, true, Collections.singletonList(TestUtils.newNode("url=invalid"))));
+                Arguments.of(InternetTags.class, 3301, 1166507262, true, Collections.singletonList(TestUtils.newNode("url=invalid"))),
+                Arguments.of(UnconnectedWays.UnconnectedHighways.class, 1311, -1765317246, true,
+                        Arrays.asList(TestUtils.newWay("highway=residential", new Node(LatLon.ZERO), new Node(LatLon.NORTH_POLE)),
+                                TestUtils.newWay("highway=residential", new Node(LatLon.ZERO), new Node(LatLon.SOUTH_POLE)))),
+        Arguments.of(UnconnectedWays.UnconnectedHighways.class, 1311, -1765317246, false,
+                Arrays.asList(TestUtils.newWay("highway=residential", new Node(LatLon.ZERO), new Node(LatLon.NORTH_POLE)),
+                        TestUtils.newWay("highway=residential", new Node(LatLon.ZERO), new Node(LatLon.SOUTH_POLE)))));
     }
 
@@ -47 +61 @@
     void testCodeCompatibility(Class<? extends Test> testClass, int originalCode, int expectedCode,
                                boolean switchOver, List<OsmPrimitive> primitiveCollection) throws ReflectiveOperationException {
+        // Create the data layer and add it to the layer manager -- needed if the test looks for an active dataset
+        final DataSet ds = new DataSet();
+        primitiveCollection.forEach(ds::addPrimitiveRecursive);
+        for (OsmPrimitive primitive : ds.allPrimitives()) {
+            if (primitive.isNew()) {
+                primitive.setOsmId(-primitive.getUniqueId(), 1);
+            }
+        }
+        final OsmDataLayer layer = new OsmDataLayer(ds, "testCodeCompatibility", null);
+        MainApplication.getLayerManager().addLayer(layer);
         // Ensure that this test always works
         TestError.setUpdateErrorCodes(switchOver);
@@ -56 +80 @@
         test.endTest();
         assertFalse(test.getErrors().isEmpty());
-        assertEquals(1, test.getErrors().size());
+        final int expectedIssues;
+        if (InternetTags.class.equals(testClass)) {
+            expectedIssues = 1;
+        } else if (UnconnectedWays.UnconnectedHighways.class.equals(testClass)) {
+            expectedIssues = 2;
+        } else {
+            expectedIssues = Integer.MIN_VALUE;
+        }
+        assertEquals(expectedIssues, test.getErrors().size());
         final TestError testError = test.getErrors().get(0);
         final String ignoreGroup = testError.getIgnoreGroup();
@@ -80 +112 @@
             OsmValidator.clearIgnoredErrors();
             OsmValidator.addIgnoredError(ignore);
+            OsmValidator.saveIgnoredErrors();
             // Add the ignored error
             assertTrue(testError.updateIgnored());