Skip to content

Commit

Permalink
[quarkusio#36582] Fix bug: TransactionalUniAsserter never fails
Browse files Browse the repository at this point in the history
TransactionalUniAsserterTestMethodInvoker is a subclass of
RunOnVertxContextTestMethodInvoker.

The problem is that there were two separate pointers keeping
track of the asserter in the superclass and in the subclass.
This lead to only one pointer being initialized and, if the wrong
pointer was null-checked, the asserter was ignored causing the test
to never fail.

This commit fixes the issue by keeping only one reference to the
asserter in the superclass.
  • Loading branch information
DavideD authored and holly-cummins committed Feb 8, 2024
1 parent cde884e commit 7cac790
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,14 @@

public class TransactionalUniAsserterTestMethodInvoker extends RunOnVertxContextTestMethodInvoker {

private UniAsserter uniAsserter;

@Override
public boolean handlesMethodParamType(String paramClassName) {
return TransactionalUniAsserter.class.getName().equals(paramClassName);
}

@Override
public Object methodParamInstance(String paramClassName) {
if (!handlesMethodParamType(paramClassName)) {
throw new IllegalStateException(
"TransactionalUniAsserterTestMethodInvoker does not handle '" + paramClassName + "' method param types");
}
uniAsserter = new TransactionalUniAsserter(new DefaultUniAsserter());
return uniAsserter;
protected UniAsserter createUniAsserter() {
return new TransactionalUniAsserter(new DefaultUniAsserter());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ public DefaultUniAsserter() {
this.data = new ConcurrentHashMap<>();
}

@Override
public Uni<?> asUni() {
return execution;
}

@SuppressWarnings("unchecked")
@Override
public <T> UniAsserter assertThat(Supplier<Uni<T>> uni, Consumer<T> asserter) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

public class RunOnVertxContextTestMethodInvoker implements TestMethodInvoker {

private DefaultUniAsserter uniAsserter;
private UniAsserter uniAsserter;

@Override
public boolean handlesMethodParamType(String paramClassName) {
Expand All @@ -32,12 +32,16 @@ public boolean handlesMethodParamType(String paramClassName) {
public Object methodParamInstance(String paramClassName) {
if (!handlesMethodParamType(paramClassName)) {
throw new IllegalStateException(
"RunOnVertxContextTestMethodInvoker does not handle '" + paramClassName + "' method param types");
this.getClass().getName() + " does not handle '" + paramClassName + "' method param types");
}
uniAsserter = new DefaultUniAsserter();
uniAsserter = createUniAsserter();
return uniAsserter;
}

protected UniAsserter createUniAsserter() {
return new DefaultUniAsserter();
}

@Override
public boolean supportsMethod(Class<?> originalTestClass, Method originalTestMethod) {
return hasSupportedAnnotation(originalTestClass, originalTestMethod)
Expand Down Expand Up @@ -152,11 +156,11 @@ public void run() {
private final Object testInstance;
private final Method targetMethod;
private final List<Object> methodArgs;
private final DefaultUniAsserter uniAsserter;
private final UniAsserter uniAsserter;
private final CompletableFuture<Object> future;

public RunTestMethodOnVertxEventLoopContextHandler(Object testInstance, Method targetMethod, List<Object> methodArgs,
DefaultUniAsserter uniAsserter, CompletableFuture<Object> future) {
UniAsserter uniAsserter, CompletableFuture<Object> future) {
this.testInstance = testInstance;
this.future = future;
this.targetMethod = targetMethod;
Expand Down Expand Up @@ -184,7 +188,7 @@ private void doRun(Runnable onTerminate) {
try {
Object testMethodResult = targetMethod.invoke(testInstance, methodArgs.toArray(new Object[0]));
if (uniAsserter != null) {
uniAsserter.execution.subscribe().with(new Consumer<Object>() {
uniAsserter.asUni().subscribe().with(new Consumer<Object>() {
@Override
public void accept(Object o) {
onTerminate.run();
Expand All @@ -209,19 +213,16 @@ public void accept(Throwable t) {
}

public static class RunTestMethodOnVertxBlockingContextHandler implements Handler<Promise<Object>> {
private static final Runnable DO_NOTHING = new Runnable() {
@Override
public void run() {
}
private static final Runnable DO_NOTHING = () -> {
};

private final Object testInstance;
private final Method targetMethod;
private final List<Object> methodArgs;
private final DefaultUniAsserter uniAsserter;
private final UniAsserter uniAsserter;

public RunTestMethodOnVertxBlockingContextHandler(Object testInstance, Method targetMethod, List<Object> methodArgs,
DefaultUniAsserter uniAsserter) {
UniAsserter uniAsserter) {
this.testInstance = testInstance;
this.targetMethod = targetMethod;
this.methodArgs = methodArgs;
Expand All @@ -248,7 +249,7 @@ private void doRun(Promise<Object> promise, Runnable onTerminate) {
try {
Object testMethodResult = targetMethod.invoke(testInstance, methodArgs.toArray(new Object[0]));
if (uniAsserter != null) {
uniAsserter.execution.subscribe().with(new Consumer<Object>() {
uniAsserter.asUni().subscribe().with(new Consumer<Object>() {
@Override
public void accept(Object o) {
onTerminate.run();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,4 +144,9 @@ public interface UniAsserter {
* Clear the test data.
*/
void clearData();

/**
* @return a {@link Uni} representing the operations pipeline up to this point
*/
Uni<?> asUni();
}
Original file line number Diff line number Diff line change
Expand Up @@ -190,4 +190,8 @@ public void clearData() {
delegate.clearData();
}

@Override
public Uni<?> asUni() {
return delegate.asUni();
}
}

0 comments on commit 7cac790

Please sign in to comment.