added test for imperial unit parsing

[kaikreuzer]

Signed-off-by: Kai Kreuzer kai@openhab.org

[kaikreuzer]

@htreu Any idea why this test fails?

[htreu]

when executing the tests in the IDE this is the first code to be called. The unit symbol "°F" is added to the parser in the static block of ImperialUnits.
The static block is executed when the class loader first loads the class, which is not needed here until is(...) is called. This is too late for the parser to acknowledge "°F" for a valid unit symbol.
Add the following setup method to the test:

@Before
public void setup() {
    @SuppressWarnings("unused")
    Unit<Temperature> fahrenheit = ImperialUnits.FAHRENHEIT;
}

See https://github.com/eclipse/smarthome/blob/5e24e27e6d83ec0594a8a5c2ad60176a799f20d7/bundles/core/org.eclipse.smarthome.core.test/src/test/java/org/eclipse/smarthome/core/library/types/QuantityTypeTest.java#L45 for reference.

[kaikreuzer]

So do we make sure that the static block of ImperialUnits is executed before any calls from the productive code are done? Or do we merely hope that someone will have accessed that class once before we hit any code that needs it?

[kaikreuzer]

In short: I do not understand why the test should be fixed, it looks like absolutely valid client code.

[htreu]

Yes, we rely on the I18nProviderImpl to be present and running. Not nice, I agree.

[kaikreuzer]

Wouldn't that be one of the few valid use cases for a bundle activator?

[kaikreuzer]

It would not help for the test of cause.

Why not? If the bundle uses lazy activation, it should work.

@maggu2810 Wdyt, does this all make sense what I am suggesting here?

[htreu]

This would instantly turn all the plain test cases into OSGi tests. Not nice.

[maggu2810]

Another option would be something like that:

diff --git a/bundles/core/org.eclipse.smarthome.core/src/main/java/org/eclipse/smarthome/core/library/unit/StaticInitializing.java b/bundles/core/org.eclipse.smarthome.core/src/main/java/org/eclipse/smarthome/core/library/unit/StaticInitializing.java
new file mode 100644
index 0000000000..426aa0d585
--- /dev/null
+++ b/bundles/core/org.eclipse.smarthome.core/src/main/java/org/eclipse/smarthome/core/library/unit/StaticInitializing.java
@@ -0,0 +1,14 @@
+package org.eclipse.smarthome.core.library.unit;
+
+public class StaticInitializing {
+
+    static {
+        ImperialUnits.getInstance();
+        SIUnits.getInstance();
+        new SmartHomeUnits();
+    }
+
+    public static void init() {
+        // A simple static method to ensure the static block of the class is executed once.
+    }
+}
diff --git a/bundles/core/org.eclipse.smarthome.core/src/main/java/org/eclipse/smarthome/core/types/util/UnitUtils.java b/bundles/core/org.eclipse.smarthome.core/src/main/java/org/eclipse/smarthome/core/types/util/UnitUtils.java
index f020531751..e570da328a 100644
--- a/bundles/core/org.eclipse.smarthome.core/src/main/java/org/eclipse/smarthome/core/types/util/UnitUtils.java
+++ b/bundles/core/org.eclipse.smarthome.core/src/main/java/org/eclipse/smarthome/core/types/util/UnitUtils.java
@@ -26,6 +26,7 @@ import org.eclipse.jdt.annotation.Nullable;
 import org.eclipse.smarthome.core.library.unit.ImperialUnits;
 import org.eclipse.smarthome.core.library.unit.SIUnits;
 import org.eclipse.smarthome.core.library.unit.SmartHomeUnits;
+import org.eclipse.smarthome.core.library.unit.StaticInitializing;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -42,6 +43,10 @@ import tec.uom.se.unit.TransformedUnit;
 @NonNullByDefault
 public class UnitUtils {
 
+    static {
+        StaticInitializing.init();
+    }
+
     private static final Logger LOGGER = LoggerFactory.getLogger(UnitUtils.class);
 
     public static final String UNIT_PLACEHOLDER = "%unit%";

We could also call StaticInitializing.init(); from an Activator (or moving that code to the Activator class)...

It depends what we want to guarantee.

[maggu2810]

@htreu What are the expected / defined entry points? Would it be enough to ensure that the initialization is done on usage of UnitUtils? Or do we need to wire other initialization paths, too?

[htreu]

Other entry points all come together in the QuantityType(String value) constructor. So adding initializing blocks to both, UnitUtils & QuantityType will be good enough.

[htreu]

With the suggestion from #6236 (comment) I created #6241.

[maggu2810]

@kaikreuzer Can you rebase your branch as #6241 has been merged now.

[kaikreuzer]

done.

[maggu2810]

Just waiting for Travis