Improved test resilience

CHANGELOG.md

@@ -66,6 +66,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - Merged SCION_DAEMON_HOST and SCION_DAEMON_PORT into a single SCION_DAEMON property.
   [#39](https://github.com/netsec-ethz/scion-java-client/pull/39)
 - Some cleanup related to hists file parser. [#42](https://github.com/netsec-ethz/scion-java-client/pull/42)
+- Added proper error when ports 30255 or 31000 are in use when running tests
+  [#50](https://github.com/netsec-ethz/scion-java-client/pull/50)
 
 ### Removed
 - Removed all code related to DatagramSockets

README.md

@@ -252,10 +252,11 @@ The following standard options are **not** supported:
 ## Configuration
 
 ### Bootstrapping / daemon
-JPAN can be used with a local daemon or in standalone mode (without daemon).
-The daemon is available if you have a [local installation of SCION](https://docs.scion.org/en/latest/dev/run.html).
-Standalone mode will directly connect to a topology server and control server, in a properly
-configured AS this should all happen automatically.
+JPAN can be used in standalone mode or with a local daemon.
+- Standalone mode will directly connect to a topology server and control server, in a properly
+  configured AS this should all happen automatically - this is the **RECOMMENDED WAY** of using this 
+  library.
+- The daemon is available if you have a [local installation of SCION](https://docs.scion.org/en/latest/dev/run.html).
 
 There are also several methods that allow specifying a local topology file, a topology server 
 address or a different DNS server with a scion NAPTR record. These are only meant for debugging. 
@@ -266,7 +267,7 @@ attempt to get network information in the following order until it succeeds:
 - For debugging: Check for bootstrap server address (if address is given)
 - For debugging: Check for DNS NAPTR record (if record entry name is given)
 - Check for to daemon
-- Check search domain (as given in /etc/resolv.conf) for topology server
+- Check search domain (as given in `/etc/resolv.conf`) for topology server
 
 The reason that the daemon is checked last is that it has a default setting (`localhost:30255`) 
 while the other options are skipped if no property or environment variable is defined. 
@@ -303,7 +304,22 @@ The certificates can be renewed by recreating the network with
 This error occurs when requesting a path with an ISD/AS code that is not
 known in the network.
 
-### Cannot find symbol javax.annotation.Generated
+### IllegalThreadStateException
+```
+[WARNING] thread Thread[grpc-default-worker-ELG-1-1,5,com.app.SimpleScmp] was interrupted but is still alive after waiting at least 15000msecs
+...
+[WARNING] Couldn't destroy threadgroup org.codehaus.mojo.exec.ExecJavaMojo$IsolatedThreadGroup[name=com.app.SimpleScmp,maxpri=10]
+java.lang.IllegalThreadStateException
+at java.lang.ThreadGroup.destroy (ThreadGroup.java:803)
+at org.codehaus.mojo.exec.ExecJavaMojo.execute (ExecJavaMojo.java:321)
+...
+```
+This can happen in your JUnit tests if the `ScionService` is not closed properly.
+To fix, close the service manually, for example by calling `ScionService.close()`.
+In normal applications this is rarely necessary because services are closed automatically by a 
+shut-down hook when the application shuts down.
+
+### "Cannot find symbol javax.annotation.Generated"
 
 ```
 Compilation failure: Compilation failure: 

src/main/java/org/scion/jpan/Constants.java

@@ -70,5 +70,11 @@ public final class Constants {
    */
   static final String DEBUG_PROPERTY_MOCK_DNS_TXT = "DEBUG_SCION_MOCK_DNS_TXT";
 
+  /**
+   * Non-public property that allows ignoring all environment variables. This is useful for running
+   * the tests on a host with a SCION installation.
+   */
+  static boolean debugIgnoreEnvironment = false;
+
   private Constants() {}
 }

src/main/java/org/scion/jpan/ScionUtil.java

@@ -28,13 +28,13 @@ public class ScionUtil {
 
   // IA represents the ISD (ISolation Domain) and AS (Autonomous System) Id of a given SCION AS.
   // The highest 16 bit form the ISD number and the lower 48 bits form the AS number.
-  private static final int ISDBits = 16;
-  private static final int MaxISD = (1 << ISDBits) - 1;
-  private static final int ASBits = 48;
-  private static final long MaxAS = (1L << ASBits) - 1L;
-  private static final int asPartBits = 16;
-  private static final int asPartBase = 16;
-  private static final int asParts = ASBits / asPartBits;
+  private static final int ISD_BITS = 16;
+  private static final int MAX_ISD = (1 << ISD_BITS) - 1;
+  private static final int AS_BITS = 48;
+  private static final long MAX_AS = (1L << AS_BITS) - 1L;
+  private static final int AS_PART_BITS = 16;
+  private static final int AS_PART_BASE = 16;
+  private static final int AS_PARTS = AS_BITS / AS_PART_BITS;
 
   /** ParseIA parses an IA from a string of the format 'isd-as'. */
   public static long parseIA(String ia) {
@@ -45,7 +45,7 @@ public static long parseIA(String ia) {
     int isd = Integer.parseUnsignedInt(parts[0], 10);
     long as = parseAS(parts[1]);
     checkLimits(isd, as);
-    return Integer.toUnsignedLong(isd) << ASBits | (as & MaxAS);
+    return Integer.toUnsignedLong(isd) << AS_BITS | (as & MAX_AS);
   }
 
   /**
@@ -59,13 +59,13 @@ private static long parseAS(String as) {
       return Integer.parseUnsignedInt(as, 10);
     }
 
-    if (parts.length != asParts) {
+    if (parts.length != AS_PARTS) {
       throw new IllegalArgumentException("Wrong number of ':' separators in value=" + as);
     }
     long parsed = 0;
-    for (int i = 0; i < asParts; i++) {
-      parsed <<= asPartBits;
-      parsed |= Long.parseUnsignedLong(parts[i], asPartBase) & 0xFFFF;
+    for (int i = 0; i < AS_PARTS; i++) {
+      parsed <<= AS_PART_BITS;
+      parsed |= Long.parseUnsignedLong(parts[i], AS_PART_BASE) & 0xFFFF;
     }
     return parsed;
   }
@@ -150,17 +150,17 @@ public static String toStringPath(RequestPath path) {
   }
 
   private static void checkLimits(int isd, long as) {
-    if (isd < 0 || isd > MaxISD) {
+    if (isd < 0 || isd > MAX_ISD) {
       throw new IllegalArgumentException("ISD out of range: " + isd);
     }
-    if (as < 0 || as > MaxAS) {
+    if (as < 0 || as > MAX_AS) {
       throw new IllegalArgumentException("AS out of range: " + as);
     }
   }
 
   public static String getPropertyOrEnv(String propertyName, String envName) {
     String value = System.getProperty(propertyName);
-    return value != null ? value : System.getenv(envName);
+    return value != null || Constants.debugIgnoreEnvironment ? value : System.getenv(envName);
   }
 
   public static String getPropertyOrEnv(String propertyName, String envName, String defaultValue) {
@@ -180,10 +180,12 @@ public static int getPropertyOrEnv(String propertyName, String envName, int defa
   }
 
   public static int extractIsd(long isdAs) {
-    return (int) (isdAs >>> ASBits);
+    return (int) (isdAs >>> AS_BITS);
   }
 
   public static long extractAs(long isdAs) {
-    return isdAs & MaxAS;
+    return isdAs & MAX_AS;
   }
+
+  private ScionUtil() {}
 }

src/test/java/org/scion/jpan/PackageVisibilityHelper.java

@@ -34,6 +34,10 @@ public class PackageVisibilityHelper {
 
   public static final String DEBUG_PROPERTY_DNS_MOCK = Constants.DEBUG_PROPERTY_MOCK_DNS_TXT;
 
+  public static void setIgnoreEnvironment(boolean flag) {
+    Constants.debugIgnoreEnvironment = flag;
+  }
+
   public static List<Daemon.Path> getPathListCS(ScionService ss, long srcIsdAs, long dstIsdAs) {
     return ss.getPathListCS(srcIsdAs, dstIsdAs);
   }

src/test/java/org/scion/jpan/testutil/JUnitSetUp.java

@@ -14,20 +14,49 @@
 
 package org.scion.jpan.testutil;
 
+import java.io.IOException;
+import java.net.ServerSocket;
 import org.junit.jupiter.api.extension.BeforeAllCallback;
 import org.junit.jupiter.api.extension.BeforeEachCallback;
 import org.junit.jupiter.api.extension.ExtensionContext;
 import org.scion.jpan.Constants;
+import org.scion.jpan.PackageVisibilityHelper;
 import org.scion.jpan.Scion;
 
 public class JUnitSetUp
     implements BeforeAllCallback, BeforeEachCallback, ExtensionContext.Store.CloseableResource {
   private static boolean started = false;
+  private static boolean failed = false;
 
   @Override
   public void beforeAll(ExtensionContext context) {
     if (!started) {
       started = true;
+
+      // Ignore environment variables
+      PackageVisibilityHelper.setIgnoreEnvironment(true);
+
+      // Check for daemon
+      try (ServerSocket ignored = new java.net.ServerSocket(30255)) {
+        // empty
+      } catch (IOException e) {
+        failed = true;
+        throw new IllegalStateException("JUnit tests cannot run while port 30255 is in use.");
+      }
+
+      // Check for control server
+      try (ServerSocket ignored = new java.net.ServerSocket(31000)) {
+        // empty
+      } catch (IOException e) {
+        failed = true;
+        throw new IllegalStateException("JUnit tests cannot run while port 31000 is in use.");
+      }
+
+      System.clearProperty(Constants.PROPERTY_BOOTSTRAP_TOPO_FILE);
+      System.clearProperty(Constants.PROPERTY_BOOTSTRAP_NAPTR_NAME);
+      System.clearProperty(Constants.PROPERTY_BOOTSTRAP_HOST);
+      System.clearProperty(Constants.PROPERTY_DAEMON);
+      System.clearProperty(Constants.PROPERTY_HOSTS_FILES);
       System.setProperty(Constants.PROPERTY_USE_OS_SEARCH_DOMAINS, "false");
       context.getRoot().getStore(ExtensionContext.Namespace.GLOBAL).put("SCION JUnit global", this);
     }
@@ -41,5 +70,8 @@ public void close() {
   @Override
   public void beforeEach(ExtensionContext context) {
     Scion.closeDefault();
+    if (failed) {
+      System.exit(1);
+    }
   }
 }