philippkunz commented on a change in pull request #28:
URL: https://github.com/apache/openwebbeans/pull/28#discussion_r439815496
##########
File path:
webbeans-junit5/src/main/java/org/apache/openwebbeans/junit5/internal/CdiExtension.java
##########
@@ -104,40 +112,42 @@ public void beforeAll(final ExtensionContext
extensionContext)
.peek(Supplier::get)
.filter(Objects::nonNull)
.toArray(Closeable[]::new);
+ SeContainer container = initializer.initialize();
if (reusable)
{
- reusableContainer = initializer.initialize();
+ reusableContainer = container;
Runtime.getRuntime().addShutdownHook(new Thread(
() -> doClose(reusableContainer), getClass().getName() +
"-shutdown"));
}
else
{
- container = initializer.initialize();
+ testInstanceContainer = container;
}
}
@Override
public void afterAll(final ExtensionContext extensionContext)
{
- if (container != null)
+ if (testInstanceContainer != null)
{
- doClose(container);
- container = null;
+ doClose(testInstanceContainer);
+ testInstanceContainer = null;
}
}
@Override
public void beforeEach(final ExtensionContext extensionContext)
{
- if (container == null && reusableContainer == null)
+ final SeContainer container = getContainer();
+ if (container == null)
Review comment:
As you mentioned error handling, it occurred to me, that this `if
(container == null)` looks suspicious to me. Would you mind removing it
including all duplicates I copied or what is the reason it is here?
I reason that for only the first trigger/callback which is `beforeAll` the
container could have to be initialized because it will be called always before
any other trigger/callback in the extension. If any other trigger/callback is
called, a container must have been initialized before, and therefore cannot be
null at this time.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
