Skip to content

Commit

Permalink
code review chnages
Browse files Browse the repository at this point in the history
  • Loading branch information
benjamin-confino committed Feb 20, 2024
1 parent c1dbb71 commit 12dec1a
Show file tree
Hide file tree
Showing 14 changed files with 44 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
import org.eclipse.microprofile.fault.tolerance.tck.asynctimeout.clientserver.AsyncTimeoutClient;
import org.eclipse.microprofile.fault.tolerance.tck.config.ConfigAnnotationAsset;
import org.eclipse.microprofile.fault.tolerance.tck.util.Connection;
import org.eclipse.microprofile.fault.tolerance.tck.util.TestException;
import org.eclipse.microprofile.faulttolerance.Timeout;
import org.jboss.arquillian.container.test.api.Deployment;
import org.jboss.arquillian.testng.Arquillian;
Expand Down Expand Up @@ -79,8 +78,7 @@ public static WebArchive deploy() {

JavaArchive testJar = ShrinkWrap
.create(JavaArchive.class, "ftAsyncTimeout.jar")
.addClasses(AsyncTimeoutClient.class, AsyncClassLevelTimeoutClient.class, Connection.class,
TestException.class)
.addClasses(AsyncTimeoutClient.class, AsyncClassLevelTimeoutClient.class, Connection.class)
.addAsManifestResource(config, "microprofile-config.properties")
.addAsManifestResource(EmptyAsset.INSTANCE, "beans.xml")
.as(JavaArchive.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public void testFallbackSuccess() {
Assert.assertTrue(result.contains("serviceA"),
"The message should be \"fallback for serviceA\"");
} catch (TestException ex) {
Assert.fail("serviceA should not throw a RuntimeException in testFallbackSuccess", ex);
Assert.fail("serviceA should not throw a TestException in testFallbackSuccess", ex);
}
Assert.assertEquals(fallbackClient.getCounterForInvokingServiceA(), 2,
"The execution count should be 2 (1 retry + 1)");
Expand All @@ -92,7 +92,7 @@ public void testFallbackSuccess() {
Assert.assertTrue(result.contains("serviceB"),
"The message should be \"fallback for serviceB\"");
} catch (TestException ex) {
Assert.fail("serviceB should not throw a RuntimeException in testFallbackSuccess", ex);
Assert.fail("serviceB should not throw a TestException in testFallbackSuccess", ex);
}
Assert.assertEquals(fallbackClient.getCounterForInvokingServiceB(), 3,
"The execution count should be 3 (2 retries + 1)");
Expand All @@ -115,7 +115,7 @@ public void testFallbackWithBeanSuccess() {
Assert.assertTrue(result.contains("34"),
"The message should be \"fallback for serviceA myBean.getCount()=34\"");
} catch (TestException ex) {
Assert.fail("serviceA should not throw a RuntimeException in testFallbackWithBeanSuccess", ex);
Assert.fail("serviceA should not throw a TestException in testFallbackWithBeanSuccess", ex);
}
Assert.assertEquals(fallbackWithBeanClient.getCounterForInvokingServiceA(), 2,
"The execution count should be 2 (1 retry + 1)");
Expand All @@ -128,7 +128,7 @@ public void testFallbackWithBeanSuccess() {
Assert.assertTrue(result.contains("35"),
"The message should be \"fallback for serviceB myBean.getCount()=35\"");
} catch (TestException ex) {
Assert.fail("serviceB should not throw a RuntimeException in testFallbackWithBeanSuccess", ex);
Assert.fail("serviceB should not throw a TestException in testFallbackWithBeanSuccess", ex);
}
Assert.assertEquals(fallbackWithBeanClient.getCounterForInvokingServiceB(), 3,
"The execution count should be 3 (2 retries + 1)");
Expand All @@ -146,7 +146,7 @@ public void testClassLevelFallbackSuccess() {
Assert.assertTrue(result.contains("serviceA"),
"The message should be \"fallback for serviceA\"");
} catch (TestException ex) {
Assert.fail("serviceA should not throw a RuntimeException in testFallbackSuccess", ex);
Assert.fail("serviceA should not throw a TestException in testFallbackSuccess", ex);
}
Assert.assertEquals(fallbackClassLevelClient.getCounterForInvokingServiceA(), 2,
"The execution count should be 2 (1 retry + 1)");
Expand All @@ -158,7 +158,7 @@ public void testClassLevelFallbackSuccess() {
"The message should be " + TestException.class.getName());

} catch (TestException ex) {
Assert.fail("serviceB should not throw a RuntimeException in testFallbackSuccess", ex);
Assert.fail("serviceB should not throw a TestException in testFallbackSuccess", ex);
}
Assert.assertEquals(fallbackClassLevelClient.getCounterForInvokingServiceB(), 3,
"The execution count should be 3 (2 retries + 1)");
Expand All @@ -181,7 +181,7 @@ public void testFallbacktNoTimeout() {
Assert.fail("serviceC should not throw a TimeoutException in testFallbacktNoTimeout");
} catch (TestException ex) {
// Not expected
Assert.fail("serviceC should not throw a RuntimeException in testFallbacktNoTimeout", ex);
Assert.fail("serviceC should not throw a TestException in testFallbacktNoTimeout", ex);
}

Assert.assertEquals(fallbackClient.getCounterForInvokingServiceC(), 2,
Expand All @@ -205,7 +205,7 @@ public void testFallbackTimeout() {
Assert.fail("serviceC should not throw a TimeoutException in testFallbackTimeout");
} catch (TestException ex) {
// Not Expected
Assert.fail("serviceC should not throw a RuntimeException in testFallbackTimeout", ex);
Assert.fail("serviceC should not throw a TestException in testFallbackTimeout", ex);
}

Assert.assertEquals(fallbackClient.getCounterForInvokingServiceC(), 2,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@
*/
public class RetryConditionTest extends Arquillian {

private final static String SIMULATED_EXCEPTION_MESSAGE = "Simulated error";
private final static String SIMULATED_RUNTIME_EXCEPTION_MESSAGE = "Test Exception - Simulated error";
public final static String SIMULATED_EXCEPTION_MESSAGE = "Simulated error";
public final static String SIMULATED_RUNTIME_EXCEPTION_MESSAGE = "Test Exception - Simulated error";

private @Inject RetryClientRetryOn clientForRetryOn;
private @Inject RetryClientAbortOn clientForAbortOn;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
import java.util.concurrent.TimeUnit;
import java.util.function.Supplier;

import org.eclipse.microprofile.fault.tolerance.tck.RetryConditionTest;

/*
*******************************************************************************
* Copyright (c) 2018 Contributors to the Eclipse Foundation
Expand Down Expand Up @@ -69,7 +71,7 @@ public CompletionStage<String> serviceA() {
countInvocationsServA++;
// always fail
CompletableFuture<String> future = new CompletableFuture<>();
future.completeExceptionally(new IOException("Simulated error"));
future.completeExceptionally(new IOException(RetryConditionTest.SIMULATED_EXCEPTION_MESSAGE));
return future;
}

Expand All @@ -82,7 +84,7 @@ public CompletionStage<String> serviceA() {
public CompletionStage<String> serviceBFailExceptionally(final CompletionStage future) {
countInvocationsServBFailExceptionally++;
// always fail
future.toCompletableFuture().completeExceptionally(new IOException("Simulated error"));
future.toCompletableFuture().completeExceptionally(new IOException(RetryConditionTest.SIMULATED_EXCEPTION_MESSAGE));
return future;
}

Expand All @@ -95,7 +97,7 @@ public CompletionStage<String> serviceBFailExceptionally(final CompletionStage f
public CompletionStage<String> serviceBFailException(final CompletionStage future) {
countInvocationsServBFailException++;
// always fail
throw new TestException("Simulated error");
throw new TestException(RetryConditionTest.SIMULATED_EXCEPTION_MESSAGE);
}

/**
Expand All @@ -112,7 +114,7 @@ public CompletionStage<String> serviceC() {

if (countInvocationsServC < 3) {
// fail 2 first invocations
future.completeExceptionally(new IOException("Simulated error"));
future.completeExceptionally(new IOException(RetryConditionTest.SIMULATED_EXCEPTION_MESSAGE));
} else {
future.complete("Success");
}
Expand All @@ -133,7 +135,7 @@ public CompletionStage<String> serviceD() {
if (countInvocationsServD < 3) {
// fail 2 first invocations
return CompletableFuture.supplyAsync(doTask(null), executor)
.thenCompose(s -> CompletableFuture.supplyAsync(doTask("Simulated error"), executor));
.thenCompose(s -> CompletableFuture.supplyAsync(doTask(RetryConditionTest.SIMULATED_EXCEPTION_MESSAGE), executor));
} else {
return CompletableFuture.supplyAsync(doTask(null), executor)
.thenCompose(s -> CompletableFuture.supplyAsync(doTask(null), executor));
Expand All @@ -153,7 +155,7 @@ public CompletionStage<String> serviceE() {

// always fail
return CompletableFuture.supplyAsync(doTask(null), executor)
.thenCompose(s -> CompletableFuture.supplyAsync(doTask("Simulated error"), executor));
.thenCompose(s -> CompletableFuture.supplyAsync(doTask(RetryConditionTest.SIMULATED_EXCEPTION_MESSAGE), executor));
}

/**
Expand All @@ -170,7 +172,7 @@ public CompletionStage<String> serviceF() {
if (countInvocationsServF < 3) {
// fail 2 first invocations
return CompletableFuture.supplyAsync(doTask(null), executor)
.thenCombine(CompletableFuture.supplyAsync(doTask("Simulated error"), executor),
.thenCombine(CompletableFuture.supplyAsync(doTask(RetryConditionTest.SIMULATED_EXCEPTION_MESSAGE), executor),
(s, s2) -> s + " then " + s2);
} else {
return CompletableFuture.supplyAsync(doTask(null), executor)
Expand All @@ -192,7 +194,7 @@ public CompletionStage<String> serviceG() {
countInvocationsServG++;
// always fail
return CompletableFuture.supplyAsync(doTask(null), executor)
.thenCombine(CompletableFuture.supplyAsync(doTask("Simulated error"), executor),
.thenCombine(CompletableFuture.supplyAsync(doTask(RetryConditionTest.SIMULATED_EXCEPTION_MESSAGE), executor),
(s, s2) -> s + " then " + s2);

}
Expand All @@ -208,7 +210,7 @@ public CompletionStage<String> serviceH() {
countInvocationsServH++;
// fails twice
if (countInvocationsServH < 3) {
throw new TestException("Simulated error");
throw new TestException(RetryConditionTest.SIMULATED_EXCEPTION_MESSAGE);
}

CompletableFuture<String> future = new CompletableFuture<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import org.eclipse.microprofile.faulttolerance.Timeout;
import org.eclipse.microprofile.faulttolerance.exceptions.CircuitBreakerOpenException;
import org.eclipse.microprofile.faulttolerance.exceptions.TimeoutException;
import org.testng.Assert;

import jakarta.enterprise.context.RequestScoped;

Expand Down Expand Up @@ -94,7 +95,7 @@ public Future<Connection> serviceC() {

try {
Thread.sleep(TCKConfig.getConfig().getTimeoutInMillis(5000));
throw new TestException("Timeout did not interrupt");
Assert.fail("Timeout did not interrupt");
} catch (InterruptedException e) {
// expected
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.eclipse.microprofile.faulttolerance.Fallback;
import org.eclipse.microprofile.faulttolerance.Retry;
import org.eclipse.microprofile.faulttolerance.Timeout;
import org.eclipse.microprofile.fault.tolerance.tck.util.TestException;

import jakarta.enterprise.context.RequestScoped;

Expand Down Expand Up @@ -111,7 +112,7 @@ public String fallbackForServiceE(String name, Integer type) {
}

private String nameService() {
throw new RuntimeException("Connection failed");
throw new TestException("Connection failed");
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package org.eclipse.microprofile.fault.tolerance.tck.fallback.clientserver;

import org.eclipse.microprofile.faulttolerance.Fallback;
import org.eclipse.microprofile.fault.tolerance.tck.util.TestException;

import jakarta.enterprise.context.RequestScoped;

Expand Down Expand Up @@ -59,6 +60,6 @@ public String fallbackForServiceB() {
}

private String nameService() {
throw new RuntimeException("Connection failed");
throw new TestException("Connection failed");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import org.eclipse.microprofile.faulttolerance.Fallback;
import org.eclipse.microprofile.faulttolerance.Retry;
import org.eclipse.microprofile.fault.tolerance.tck.util.TestException;

import jakarta.enterprise.context.RequestScoped;

Expand Down Expand Up @@ -59,7 +60,7 @@ public String serviceB() {
}

private String nameService() {
throw new RuntimeException("Connection failed");
throw new TestException("Connection failed");
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public int getRetryCountForConnectionService() {
/**
* serviceB is configured to retry on a RuntimeException. The WritingService throws RuntimeExceptions.
*/
@Retry(abortOn = {RuntimeException.class})
@Retry(abortOn = {TestException.class})
public void serviceB() {
writingService();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@
*******************************************************************************/
package org.eclipse.microprofile.fault.tolerance.tck.visibility.retry;

import java.io.IOException;
import java.sql.Connection;

import org.eclipse.microprofile.faulttolerance.Retry;
import org.eclipse.microprofile.fault.tolerance.tck.util.TestException;

import jakarta.enterprise.context.RequestScoped;

Expand All @@ -33,13 +33,13 @@ public class BaseRetryOnClassService implements RetryService {
private int nbCalls = 0;

@Override
public Connection service() throws IOException {
public Connection service() {
nbCalls++;
return delegate();
}

protected Connection delegate() throws IOException {
throw new IOException("cannot access delegate service");
protected Connection delegate() {
throw new TestException("cannot access delegate service");
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,13 @@

import jakarta.enterprise.context.RequestScoped;

import org.eclipse.microprofile.fault.tolerance.tck.util.TestException;

@RequestScoped
@RS(RetryServiceType.BASE_ROC_RETRY_MISSING_ON_METHOD)
public class RetryOnClassServiceNoAnnotationOnOveriddenMethod extends BaseRetryOnClassService {
@Override
public Connection service() throws IOException {
public Connection service() {
return super.service();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.sql.Connection;

import org.eclipse.microprofile.faulttolerance.Retry;
import org.eclipse.microprofile.fault.tolerance.tck.util.TestException;

import jakarta.enterprise.context.RequestScoped;

Expand All @@ -31,7 +32,7 @@
@Retry(maxRetries = 4)
public class RetryOnClassServiceOverrideClassLevelMethodOverride extends BaseRetryOnClassService {
@Override
public Connection service() throws IOException {
public Connection service() {
return super.service();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.sql.Connection;

import org.eclipse.microprofile.faulttolerance.Retry;
import org.eclipse.microprofile.fault.tolerance.tck.util.TestException;

import jakarta.enterprise.context.RequestScoped;

Expand All @@ -37,7 +38,7 @@
public class RetryOnClassServiceOverrideMethodLevel extends BaseRetryOnClassService {
@Override
@Retry(maxRetries = 4)
public Connection service() throws IOException {
public Connection service() {
return super.service();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@
*******************************************************************************/
package org.eclipse.microprofile.fault.tolerance.tck.visibility.retry;

import java.io.IOException;

import org.eclipse.microprofile.fault.tolerance.tck.util.TestException;
import org.jboss.arquillian.container.test.api.Deployment;
import org.jboss.arquillian.testng.Arquillian;
Expand Down Expand Up @@ -265,15 +263,15 @@ private void checkServiceCall(int nbExpectedRetries, RetryService service, Strin
service.service();
Assert.fail(String.format("in %s#%s service() should have failed",
RetryVisibilityTest.class.getSimpleName(), testName));
} catch (IOException re) {
} catch (TestException re) {
Assert.assertEquals(
service.getNumberOfServiceCalls(),
expectedNbCalls,
String.format("in %s#%s service() should have been called exactly %d times",
RetryVisibilityTest.class.getSimpleName(),
testName,
expectedNbCalls));
} catch (TestException ex) {
} catch (Exception ex) {
Assert.fail(String.format("no %s exception should have been thrown in %s#%s",
ex.getClass().getName(),
RetryVisibilityTest.class.getSimpleName(),
Expand Down

0 comments on commit 12dec1a

Please sign in to comment.