Repository: eclipse/microprofile-lra (java)

Issue:
Timeout adjustment should be available at all places when timeout is used
This is a follow-up issue to my previous one where I was asking for TCK changing timeout based of the property `lra.tck.timeout.factor` not only in-container test methods but in `TckTests#timeLimit` bean which is `LraResource` one.

But there are few more places where the timeout factor is still not used. One is the `LraResource` bean and the other is recovery tests. The recovery tests is a little bit more complicated as they are run outside of the container.

Pull request:
[lra#312] adjust sleep time by timeout factor in TckRecoveryTests
fixes #312 

I propose a change on timeout handling in the TCK as I assume the timeout factor should be considered at all places where `Thread.sleep` is used.

@xstefank @mmusgrov @rdebusscher : if you may please take a look if such a change is fine from your perspective

Patch:
diff --git a/tck/running_the_tck.asciidoc b/tck/running_the_tck.asciidoc
index 0304ae9..3b2a458 100644
--- a/tck/running_the_tck.asciidoc
+++ b/tck/running_the_tck.asciidoc
@@ -37,6 +37,7 @@ The LRA TCK suite can be parametrized by following properties, handled in the su
   When set-up lower then the timeouts will be shorter.
   Thus on slower machines it's expected longer timeouts will be needed. For example if test expects
   some waiting time to be 10 seconds and this factor is set to `1.5` then the result waiting time is 15 seconds.
+  The timeout factor must be configured as system property because of the test limitations.
 `lra.http.recovery.host`, `lra.http.recovery.port`, `lra.http.recovery.path`::
   Hostname, port and path for the recovery endpoint that will be contacted in tests checking recovery capabilities.
 
diff --git a/tck/src/main/java/org/eclipse/microprofile/lra/tck/LraTckConfigBean.java b/tck/src/main/java/org/eclipse/microprofile/lra/tck/LraTckConfigBean.java
index ce0e48a..90a2e83 100644
--- a/tck/src/main/java/org/eclipse/microprofile/lra/tck/LraTckConfigBean.java
+++ b/tck/src/main/java/org/eclipse/microprofile/lra/tck/LraTckConfigBean.java
@@ -34,6 +34,18 @@ public class LraTckConfigBean {
      */
     private static final Long LRA_TIMEOUT_MILLIS = 50000L;
 
+    /**
+     * Name of the config property that is used as factor adjusting timeout values
+     * used in the testsuite.
+     */
+    public static final String LRA_TCK_TIMEOUT_FACTOR_PROPETY_NAME = "lra.tck.timeout.factor";
+
+    /**
+     * Name of the config property which is used to configure the TCK base url.
+     * See {@link LraTckConfigBean#tckSuiteBaseUrl}.
+     */
+    public static final String LRA_TCK_BASE_URL_PROPERTY_NAME = "lra.tck.base.url";
+
     /**
      * <p>
      * Timeout factor which adjusts waiting time and timeouts for the TCK suite.
@@ -45,7 +57,7 @@ public class LraTckConfigBean {
      * If you wish the test waits shorter time than designed
      * or the timeout is elapsed faster then set the value less than <code>1.0</code>
      */
-    @Inject @ConfigProperty(name = "lra.tck.timeout.factor", defaultValue = "1.0")
+    @Inject @ConfigProperty(name = LRA_TCK_TIMEOUT_FACTOR_PROPETY_NAME, defaultValue = "1.0")
     private double timeoutFactor;
 
     /**
@@ -54,12 +66,12 @@ public class LraTckConfigBean {
      * <p>
      * The default base URL where TCK suite is expected to be started is <code>http://localhost:8180/</code>.
      */
-    @Inject @ConfigProperty(name = "lra.tck.base.url", defaultValue = "http://localhost:8180/")
+    @Inject @ConfigProperty(name = LRA_TCK_BASE_URL_PROPERTY_NAME, defaultValue = "http://localhost:8180/")
     private String tckSuiteBaseUrl;
 
     /**
      * Adjusting the default timeout by the specified timeout factor which can be defined by user
-     * when property <code>lra.tck.timeout.factor</code> is defined.
+     * when property {@code #LRA_TCK_TIMEOUT_FACTOR_PROPETY_NAME} is defined.
      *
      * @return default timeout adjusted with timeout factor
      */
@@ -70,10 +82,19 @@ public class LraTckConfigBean {
     /**
      * Adjusting the provided value by timeout factor defined for the TCK suite.
      *
-     * @param timeout timeout value to be adjusted by 'lra.tck.timeout.factor'
+     * @param timeout timeout value to be adjusted by {@code #LRA_TCK_TIMEOUT_FACTOR_PROPETY_NAME}
      * @return value of adjusted timeout
      */
     public long adjustTimeout(long timeout) {
+        return adjustTimeout(timeout, timeoutFactor);
+    }
+
+    private long adjustTimeout(long timeout, double timeoutFactor) {
+        if (timeout < 0 || timeoutFactor < 0)  {
+            throw new IllegalArgumentException(String.format(
+                    "Provided arguments (timeout=%d, timeoutFactor=%.2f) have to be positive", timeout, timeoutFactor));
+        }
         return (long) Math.ceil(timeout * timeoutFactor);
     }
+
 }
diff --git a/tck/src/main/java/org/eclipse/microprofile/lra/tck/participant/api/LraResource.java b/tck/src/main/java/org/eclipse/microprofile/lra/tck/participant/api/LraResource.java
index 171b43f..dde3849 100644
--- a/tck/src/main/java/org/eclipse/microprofile/lra/tck/participant/api/LraResource.java
+++ b/tck/src/main/java/org/eclipse/microprofile/lra/tck/participant/api/LraResource.java
@@ -419,7 +419,7 @@ public class LraResource {
     @GET
     @Path(TIME_LIMIT)
     @Produces(MediaType.APPLICATION_JSON)
-    @LRA(value = LRA.Type.REQUIRED, timeLimit = 100, timeUnit = ChronoUnit.MILLIS)
+    @LRA(value = LRA.Type.REQUIRED, timeLimit = 500, timeUnit = ChronoUnit.MILLIS)
     public Response timeLimit(@HeaderParam(LRA_HTTP_CONTEXT_HEADER) URI lraId) {
         assertHeaderPresent(lraId, LRA_HTTP_CONTEXT_HEADER);
 
@@ -427,7 +427,7 @@ public class LraResource {
 
         try {
             // sleep longer time than specified in the attribute 'timeLimit'
-            Thread.sleep(configBean.adjustTimeout(300));
+            Thread.sleep(configBean.adjustTimeout(1000));
         } catch (InterruptedException e) {
             LOGGER.log(Level.FINE, "Interrupted because time limit elapsed", e);
         }
@@ -447,7 +447,7 @@ public class LraResource {
         activityStore.add(new Activity(lraId));
 
         try {
-            Thread.sleep(1000); // sleep for longer than specified in the timeLimit annotation attribute
+            Thread.sleep(configBean.adjustTimeout(1000)); // sleep for longer than specified in the timeLimit annotation attribute
             // force the implementation to notice that the LRA should have timed out
             lraTestService.waitForCallbacks(lraId);
             // the next request should fail with a 412 code since the LRA should no longer be active


Test patch:
diff --git a/tck/src/main/java/org/eclipse/microprofile/lra/tck/TckRecoveryTests.java b/tck/src/main/java/org/eclipse/microprofile/lra/tck/TckRecoveryTests.java
index ec308d9..5822b5e 100644
--- a/tck/src/main/java/org/eclipse/microprofile/lra/tck/TckRecoveryTests.java
+++ b/tck/src/main/java/org/eclipse/microprofile/lra/tck/TckRecoveryTests.java
@@ -186,7 +186,7 @@ public class TckRecoveryTests {
         // Then wait for the short delay to actually perform the cancellation while the service is still down.
         // Compensate should be attempted to be called while the participant service is down
         try {
-            Thread.sleep(RecoveryResource.LRA_TIMEOUT);
+            Thread.sleep(adjustTimeoutByDefaultFactor(RecoveryResource.LRA_TIMEOUT));
         } catch (InterruptedException e) {
             LOG.log(Level.SEVERE, "Sleep LRA timeout interrupted", e);
             Assert.fail("Sleeping LRA timeout " + RecoveryResource.LRA_TIMEOUT + " was interrupted: " + e.getMessage());
@@ -221,4 +221,13 @@ public class TckRecoveryTests {
         Assert.assertTrue("Expecting the metric " + metricType + " callback was called",
                 metricNumber >= 1);
     }
+
+    /**
+     * A helper method which is capable to adjust timeout value
+     * by factor obtained from system property {@code LraTckConfigBean#LRA_TCK_TIMEOUT_FACTOR_PROPETY_NAME}.
+     */
+    private long adjustTimeoutByDefaultFactor(long timeout) {
+        String timeoutFactor = System.getProperty(LraTckConfigBean.LRA_TCK_TIMEOUT_FACTOR_PROPETY_NAME, "1.0");
+        return (long) Math.ceil(timeout * Double.parseDouble(timeoutFactor));
+    }
 }
diff --git a/tck/src/main/java/org/eclipse/microprofile/lra/tck/TckTests.java b/tck/src/main/java/org/eclipse/microprofile/lra/tck/TckTests.java
index 180ec1d..6dbda37 100644
--- a/tck/src/main/java/org/eclipse/microprofile/lra/tck/TckTests.java
+++ b/tck/src/main/java/org/eclipse/microprofile/lra/tck/TckTests.java
@@ -318,8 +318,7 @@ public class TckTests extends TckTestBase {
                 .request()
                 .get();
 
-        URI lraId = URI.create(response.readEntity(String.class));
-        response.close();
+        URI lraId = URI.create(checkStatusReadAndCloseResponse(Response.Status.OK, response, resourcePath));
 
         // Note that the timeout firing will cause the implementation to compensate
         // the LRA so it may no longer exist