Switch to JUnit and add tests

JUnit is the only supported test framework for JOSM plugins. https://josm.openstreetmap.de/ticket/23219

Author: gabortim

CHANGELOG.md

@@ -4,6 +4,15 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [1.1.1] - 2023-10-XX
+### Added
+- More tests, including some regression tests
+### Changed
+- Update Gradle to 8.4
+- Switch the test framework from TestNG to JUnit because of upstream dependency
+- Fixed a typo in a warning message
+- Fixed duplicate warning for the same autofixable issue (bad separator and beatutifiable)
+
 ## [1.1.0] - 2023-10-05
 ### Added
 - New validator message in case no (auto-fixable) formatting issue found

build.gradle.kts

@@ -74,27 +74,31 @@ fun getGitHash(): String {
 }
 
 dependencies {
+    testImplementation("org.junit.jupiter:junit-jupiter:5.8.1")
     packIntoJar(kotlin("stdlib"))
 
     implementation("org.openstreetmap.josm.plugins:libphonenumber:8.+") { isChanging = true }
 
-    testImplementation("org.testng:testng") {
-        version {
-            strictly("7.5")
-            because("TestNG 7.6 and up requires Java 11 or newer")
-        }
-    }
+    // fix test runtime issue
+    // https://mvnrepository.com/artifact/org.wiremock/wiremock-standalone
+    testImplementation("org.wiremock:wiremock-standalone:3.2.0")
+    testImplementation(kotlin("reflect"))
+
+    val junit = "5.9.3"
+    testRuntimeOnly("org.junit.jupiter:junit-jupiter-engine:${junit}")
+    testImplementation("org.junit.jupiter:junit-jupiter-api:${junit}")
+    testImplementation("org.openstreetmap.josm:josm-unittest:SNAPSHOT") { isChanging = true }
 }
 
 tasks.test {
-    useTestNG()
+    useJUnitPlatform()
     finalizedBy(tasks.jacocoTestReport) // report is always generated after tests run
 }
 
 tasks.jacocoTestReport {
     dependsOn(tasks.test) // tests are required to run before generating the report
     reports {
-        xml.required.set(true)
+        xml.required = true
     }
 }
 

src/main/kotlin/com/github/gabortim/phonenumber/action/PhoneNumberDownloadAction.kt

@@ -1,5 +1,6 @@
 package com.github.gabortim.phonenumber.action
 
+import com.github.gabortim.phonenumber.task.PhoneNumberDownloadTask
 import org.openstreetmap.josm.actions.JosmAction
 import org.openstreetmap.josm.data.Bounds
 import org.openstreetmap.josm.gui.MainApplication

src/main/kotlin/com/github/gabortim/phonenumber/action/PhoneNumberDownloadTask.kt renamed to src/main/kotlin/com/github/gabortim/phonenumber/task/PhoneNumberDownloadTask.kt

@@ -1,4 +1,4 @@
-package com.github.gabortim.phonenumber.action
+package com.github.gabortim.phonenumber.task
 
 import com.github.gabortim.phonenumber.test.PhoneNumberValidator
 import org.openstreetmap.josm.data.Bounds

src/main/kotlin/com/github/gabortim/phonenumber/test/PhoneNumber.kt

@@ -159,6 +159,7 @@ class PhoneNumber(
                             if (number != formatted.first)
                                 isFormatted = true
                         }
+
                         FailReason.UNUSUAL_CHARS -> unusualChars.add(number)
                         FailReason.INVALID -> invalid.add(number)
                         FailReason.TOO_SHORT -> tooShort.add(number)
@@ -240,12 +241,15 @@ class PhoneNumber(
     }
 
     /**
-     * Replies true if any of the phone numbers has separator issue.
+     * Returns true if any of the phone numbers has separator issue.
      */
     private fun hasWrongSeparator(): Boolean {
         return badSeparator.isNotEmpty()
     }
 
+    /**
+     * Returns true if the primitive is fixable.
+     */
     fun isFixable(): Boolean {
         return hasDuplicates() || hasPremiumNumber() || hasWrongSeparator() || isFormatted || isBeautifyable || hasSwitchedClass || hasSchemaChange
     }
@@ -267,7 +271,7 @@ class PhoneNumber(
         if (isFormatted)
             description.add(tr("not in E.123 format"))
         else if (isBeautifyable)
-            description.add(tr("beatuifyable value"))
+            description.add(tr("beautifiable value"))
         if (hasSwitchedClass)
             description.add(tr("inappropriate key"))
         if (hasSchemaChange)

src/main/kotlin/com/github/gabortim/phonenumber/test/PhoneNumberValidator.kt

@@ -101,17 +101,6 @@ class PhoneNumberValidator : TagTest(
         parsedNumbers = PhoneNumber(primitive, getIso3166Alpha2Code(primitive), forceContactSchemeProperty.get())
 
         /// non autofixable errors
-        for (key in parsedNumbers.badSeparator) {
-            errors.add(
-                TestError.builder(this, Severity.ERROR, BAD_SEPARATOR)
-                    .message(
-                        tr("Phone number invalid"),
-                        tr("wrong separator used in {0} key", key)
-                    )
-                    .primitives(primitive)
-                    .build()
-            )
-        }
         for (value in parsedNumbers.invalid) {
             errors.add(
                 TestError.builder(this, Severity.ERROR, PARSE_ERROR)
@@ -169,7 +158,19 @@ class PhoneNumberValidator : TagTest(
         }
 
         /// autofixable warnings
-        if (parsedNumbers.isFixable()) {
+        for (key in parsedNumbers.badSeparator) {
+            errors.add(
+                TestError.builder(this, Severity.ERROR, BAD_SEPARATOR)
+                    .message(
+                        tr("Phone number invalid"),
+                        tr("wrong separator used in {0} key", key)
+                    )
+                    .primitives(primitive)
+                    .build()
+            )
+        }
+        // avoid duplicate warning
+        if (parsedNumbers.isFixable() && parsedNumbers.badSeparator.isEmpty()) {
             errors.add(
                 TestError.builder(this, Severity.WARNING, MULTI)
                     .message(

src/main/kotlin/com/github/gabortim/phonenumber/tool/NumberTools.kt

@@ -57,7 +57,7 @@ internal object NumberTools {
      * @return Split string in an array
      */
     fun splitAndStrip(value: String): Collection<String> {
-        return value.split(Regex("[,;]"))
+        return value.split(PatternUtils.compile("[,;]"))
             .filter(String::isNotBlank)
             .map(String::trim)
     }

src/test/kotlin/com/github/gabortim/phonenumber/test/PhoneNumberTest.kt

@@ -0,0 +1,34 @@
+package com.github.gabortim.phonenumber.test
+
+import org.junit.jupiter.api.Assertions.assertFalse
+import org.junit.jupiter.api.Assertions.assertTrue
+import org.junit.jupiter.api.Test
+import org.openstreetmap.josm.data.coor.LatLon
+import org.openstreetmap.josm.data.osm.Node
+import org.openstreetmap.josm.data.osm.TagMap
+import org.openstreetmap.josm.testutils.annotations.BasicPreferences
+
+@BasicPreferences
+/**
+ * Test for PhoneNumber.
+ */
+class PhoneNumberTest {
+    @Test
+    fun isFixable() {
+        val node = Node(LatLon.ZERO)
+        val tags = TagMap("phone", "+36 66 390 686")
+        node.setKeys(tags)
+
+        assertFalse(PhoneNumber(node, "HU", false).isFixable())
+        assertTrue(PhoneNumber(node, "HU", true).isFixable())
+    }
+
+    @Test
+    fun testDuplicateDetection() {
+        val node = Node(LatLon.ZERO)
+        val tags = TagMap("contact:mobile", "+36 30 000 0000;+36 30 000 0000")
+        node.setKeys(tags)
+
+        assertTrue(PhoneNumber(node, "HU", true).isFixable())
+    }
+}