You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomee.apache.org by GitBox <gi...@apache.org> on 2021/03/09 11:53:13 UTC
[GitHub] [tomee] rzo1 opened a new pull request #767: TOMEE-2977 - Provide a ApplicationComposer JUnit 5 Extension
rzo1 opened a new pull request #767:
URL: https://github.com/apache/tomee/pull/767
# What does this PR do?
- Adds a JUnit 5 extension, which internally uses `ApplicationComposers` for (pure) JUnit 5 testing.
- Adds some unit tests (based on existing `ApplicationComposer` tests) written in JUnit 5
- Provides an additional JUnit 5 example (based on `application-composer`)
- Upgrades Maven surefire to `2.22.1` ( `>=2.22.0` is required to run both variants of unit tests without workarounds out of the box)
# References
- https://issues.apache.org/jira/projects/TOMEE/issues/TOMEE-2977
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tomee] rzo1 commented on a change in pull request #767: TOMEE-2977 - Provide a ApplicationComposer JUnit 5 Extension
Posted by GitBox <gi...@apache.org>.
rzo1 commented on a change in pull request #767:
URL: https://github.com/apache/tomee/pull/767#discussion_r596331107
##########
File path: container/openejb-core/src/main/java/org/apache/openejb/junit/jupiter/ApplicationComposerExtension.java
##########
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit.jupiter;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.apache.openejb.testing.ApplicationComposers;
+import org.apache.openejb.testing.SingleApplicationComposerBase;
+import org.junit.jupiter.api.extension.*;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+
+public class ApplicationComposerExtension extends ApplicationComposerExtensionBase implements BeforeAllCallback, AfterAllCallback, BeforeEachCallback, AfterEachCallback {
+
+ private static final ExtensionContext.Namespace NAMESPACE = ExtensionContext.Namespace.create(ApplicationComposerExtension.class.getName());
+ private static final SingleApplicationComposerBase BASE = new SingleApplicationComposerBase();
+
+ @Override
+ public void beforeAll(ExtensionContext context) throws Exception {
+
+ if (isPerJvm(context)) {
+ BASE.start(context.getTestClass().orElse(null));
Review comment:
Alright - just added a related validation.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tomee] rzo1 commented on a change in pull request #767: TOMEE-2977 - Provide a ApplicationComposer JUnit 5 Extension
Posted by GitBox <gi...@apache.org>.
rzo1 commented on a change in pull request #767:
URL: https://github.com/apache/tomee/pull/767#discussion_r596715983
##########
File path: container/openejb-junit5/src/main/java/org/apache/openejb/junit5/ApplicationComposerPerEachExtension.java
##########
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit5;
+
+import org.junit.jupiter.api.extension.AfterAllCallback;
+import org.junit.jupiter.api.extension.AfterEachCallback;
+import org.junit.jupiter.api.extension.BeforeAllCallback;
+import org.junit.jupiter.api.extension.BeforeEachCallback;
+import org.junit.jupiter.api.extension.ExtensionContext;
+
+public class ApplicationComposerPerEachExtension extends ApplicationComposerPerXYExtensionBase implements BeforeAllCallback, AfterAllCallback, BeforeEachCallback, AfterEachCallback {
+
+ public ApplicationComposerPerEachExtension() {
+ this((Object[]) null);
+ }
+
+ public ApplicationComposerPerEachExtension(Object... modules) {
+ super(modules);
+ }
+
+ @Override
+ void validate(ExtensionContext context) {
+ //no-op
+ }
+
+ @Override
+ public void afterAll(ExtensionContext context) throws Exception {
+ addAfterEachReleaser(context);
Review comment:
it is **fishy**. good catch!
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tomee] rzo1 commented on a change in pull request #767: TOMEE-2977 - Provide a ApplicationComposer JUnit 5 Extension
Posted by GitBox <gi...@apache.org>.
rzo1 commented on a change in pull request #767:
URL: https://github.com/apache/tomee/pull/767#discussion_r590340268
##########
File path: container/openejb-core/src/main/java/org/apache/openejb/junit/jupiter/ApplicationComposerExtension.java
##########
@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit.jupiter;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.apache.openejb.testing.ApplicationComposers;
+import org.junit.jupiter.api.extension.*;
+
+public class ApplicationComposerExtension implements BeforeEachCallback, AfterEachCallback {
Review comment:
Wow. Super fast :+1:
1. I thought about it: Doesn't `ApplicationComposers` require a concrete test instance to inject values and stuff in `delegate.before(...)`? The required test instance is - afaik - not available when `beforeAll(...)` is called. However, the creation of the `delegate` might be conducted in a `beforeAll(...) call.
2. This is a good idea. I think, it can be done via `context.getRoot().getStore(...)` the `ExtensionContext`.
3. Honestly, I did not thought about this use-case. We may get additional test instances via `context.getRequiredTestInstances().getAllInstances()`. Wdyt?
4. Should be very similar to `SingleApplicationComposerRunner` - I will take a look at it.
5. Good idea. Will change it to `RunWith...` to be inline with the namings in `openejb-junit5-backward`.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tomee] rzo1 commented on a change in pull request #767: TOMEE-2977 - Provide a ApplicationComposer JUnit 5 Extension
Posted by GitBox <gi...@apache.org>.
rzo1 commented on a change in pull request #767:
URL: https://github.com/apache/tomee/pull/767#discussion_r596740918
##########
File path: container/openejb-junit5/src/main/java/org/apache/openejb/junit5/ApplicationComposerPerJVMExtension.java
##########
@@ -0,0 +1,109 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit5;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.apache.openejb.testing.Classes;
+import org.apache.openejb.testing.Component;
+import org.apache.openejb.testing.Default;
+import org.apache.openejb.testing.Jars;
+import org.apache.openejb.testing.Module;
+import org.apache.openejb.testing.SingleApplicationComposerBase;
+import org.apache.xbean.finder.ClassFinder;
+import org.junit.jupiter.api.extension.AfterAllCallback;
+import org.junit.jupiter.api.extension.AfterEachCallback;
+import org.junit.jupiter.api.extension.BeforeAllCallback;
+import org.junit.jupiter.api.extension.BeforeEachCallback;
+import org.junit.jupiter.api.extension.ExtensionContext;
+import org.junit.jupiter.api.extension.TestInstances;
+
+import java.lang.annotation.Annotation;
+import java.util.ArrayList;
+import java.util.List;
+
+import static org.apache.openejb.util.Classes.ancestors;
+
+public class ApplicationComposerPerJVMExtension extends ApplicationComposerPerXYExtensionBase implements BeforeAllCallback, AfterAllCallback, BeforeEachCallback, AfterEachCallback {
+
+ private static final SingleApplicationComposerBase BASE = new SingleApplicationComposerBase();
+
+ @Override
+ void validate(ExtensionContext context) {
+ if (!isPerJvm(context) && BASE.isStarted()) {
+ //XXX: Future work: We might get it to work via a JVM singleton/lock, see https://github.com/apache/tomee/pull/767#discussion_r595343572
+ throw new OpenEJBRuntimeException("Cannot run PER_JVM in combination with PER_ALL, PER_EACH or AUTO");
+ }
+
+ Class<?> clazz = context.getTestClass()
+ .orElseThrow(() -> new OpenEJBRuntimeException("Could not obtain test class from extension context"));
+
+ final List<Throwable> errors = new ArrayList<>();
+
+ ClassFinder classFinder = new ClassFinder(ancestors(clazz));
+
+ Class<? extends Annotation>[] toCheck = new Class[]{Component.class, Module.class, Classes.class, Default.class, Jars.class};
+
+ for (Class<? extends Annotation> annotation : toCheck) {
+ if (classFinder.isAnnotationPresent(annotation)) {
+ errors.add(new Exception("@" + annotation.getName() + " is not allowed with @Application in PER_JVM mode"));
+ }
+ }
+
+ if (!errors.isEmpty()) {
+ throw new OpenEJBRuntimeException(errors.toString());
+ }
+ }
+
+ @Override
+ public void beforeAll(ExtensionContext context) throws Exception {
+ super.beforeAll(context);
+
+ BASE.start(context.getTestClass().orElse(null));
+ if (isPerClassLifecycle(context)) {
+ doInject(context);
+ }
+ }
+
+ @Override
+ public void beforeEach(ExtensionContext context) throws Exception {
+ if (isPerMethodLifecycle(context)) {
+ doInject(context);
+ }
+ }
+
+ private void doInject(final ExtensionContext extensionContext) {
Review comment:
From my understanding of the code:
`ApplicationComposers.before(...)` starts a container, deploys the app and does the injection internally (in `doStart(...)`) ? In the `PER_JVM`-case, we have this single app composer thing, which runs over the full JVM lifetime and is required to inject the `@Application` into (new) test instances via `doInject(...)`.
So my question would be: What do you mean exactly? Can you add some more details, @rmannibucau ?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tomee] rmannibucau commented on a change in pull request #767: TOMEE-2977 - Provide a ApplicationComposer JUnit 5 Extension
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on a change in pull request #767:
URL: https://github.com/apache/tomee/pull/767#discussion_r590508449
##########
File path: container/openejb-core/src/main/java/org/apache/openejb/junit/jupiter/ApplicationComposerExtension.java
##########
@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit.jupiter;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.apache.openejb.testing.ApplicationComposers;
+import org.junit.jupiter.api.extension.*;
+
+public class ApplicationComposerExtension implements BeforeEachCallback, AfterEachCallback {
Review comment:
The split of an app composer app in multiple class is actually what you use ;). The constructor of ApplicationComposers takes N objects, just pass all test classes if you want a single deployment for all tests and enable each test to deploy part of the app.
About the lifecycle it is not 1-1 with test instance lifecycle but the same idea for the container lifecycle (but you can have one test instance per test and one container instance for all tests of a single class when you want to reset another extension data/field instances for example). We can align on the default if nothing is set (so add to the enum an AUTO mode ;)) but must be configurable.
About SingleApplicationComposerExtension i was thinking to https://github.com/apache/tomee/blob/master/tomee/tomee-embedded/src/main/java/org/apache/tomee/embedded/junit/TomEEEmbeddedSingleRunner.java or the same of meecrowave, when the app is fully modelized and you don't test a lib you only use that cause the perf boost is really impressive compared to starting one container per test.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tomee] rmannibucau commented on a change in pull request #767: TOMEE-2977 - Provide a ApplicationComposer JUnit 5 Extension
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on a change in pull request #767:
URL: https://github.com/apache/tomee/pull/767#discussion_r590380714
##########
File path: container/openejb-core/src/main/java/org/apache/openejb/junit/jupiter/ApplicationComposerExtension.java
##########
@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit.jupiter;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.apache.openejb.testing.ApplicationComposers;
+import org.junit.jupiter.api.extension.*;
+
+public class ApplicationComposerExtension implements BeforeEachCallback, AfterEachCallback {
Review comment:
1. to inject anything yes but you don't always have to inject. That said point was the opposite: what if you split your test definition in multiple classes? What if you use a dynamic test? I suspect it can be worth a `@ApplicationComposer(mode = [PER_JVM|PER_ALL|PER_EACH])` - note that per_all/per_each are not great but per_class/per_method are not accurate with junit5 so feel free to find a better name, just sharing the concept I have in mind.
2. no getRoot(), ensure to align the scope on the actual one: ` context.getStore(NAMESPACE)` with `private static final ExtensionContext.Namespace NAMESPACE = ExtensionContext.Namespace.create(TheExtension.class.getName());` probably
3. right but do not used required accessor but optional one ;)
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tomee] rzo1 commented on a change in pull request #767: TOMEE-2977 - Provide a ApplicationComposer JUnit 5 Extension
Posted by GitBox <gi...@apache.org>.
rzo1 commented on a change in pull request #767:
URL: https://github.com/apache/tomee/pull/767#discussion_r595489470
##########
File path: container/openejb-core/pom.xml
##########
@@ -580,6 +580,22 @@
<groupId>org.apache.xbean</groupId>
<artifactId>xbean-bundleutils</artifactId>
</dependency>
+ <dependency>
Review comment:
I think it would be (a) cleaner and (b) could allow user's to exclude the compile dependencies towards junit4 - so +1 for `openejb-junit5`.
Could also include the classes from `openejb-junit5-backward` (was not released yet -> so no problem in renaming / dropping it), so we have all junit 5 related things in one module. wdyt?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tomee] rzo1 commented on pull request #767: TOMEE-2977 - Provide a ApplicationComposer JUnit 5 Extension
Posted by GitBox <gi...@apache.org>.
rzo1 commented on pull request #767:
URL: https://github.com/apache/tomee/pull/767#issuecomment-801920909
Should now contain the proposed fixes / enhancements.
Tests are now executed in an additional phase. Let's wait if we receive some additional feedback.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tomee] rzo1 commented on a change in pull request #767: TOMEE-2977 - Provide a ApplicationComposer JUnit 5 Extension
Posted by GitBox <gi...@apache.org>.
rzo1 commented on a change in pull request #767:
URL: https://github.com/apache/tomee/pull/767#discussion_r592724306
##########
File path: container/openejb-core/src/main/java/org/apache/openejb/junit/jupiter/ApplicationComposerExtension.java
##########
@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit.jupiter;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.apache.openejb.testing.ApplicationComposers;
+import org.junit.jupiter.api.extension.*;
+
+public class ApplicationComposerExtension implements BeforeEachCallback, AfterEachCallback {
Review comment:
The current impl relies on the test's life cycle to start a container or perform related injections.
Atm the impl contains annotations and extensions for
- `RunWithSingleApplicationComposer`
- `RunWithApplicationComposer`
as well as a
- `TomEEEmbeddedExtension`
From my understanding, the proposed `mode` is solely responsible to configure how (or when) the underlying container is started? I.e. one container to run all tests within a single test class (PER_ALL), one container for each test method of one test class (PER_EACH) and one container to run multiple test classes (PER_JVM), right? Is this the same idea, which is currently conducted through different JUnit4 runners?
From my understanding of the current JUnit implementation, the `ApplicationComposer` seems to be something similar to the PER_ALL and the `EmbeddedTomEERunner` / `SingleApplicationComposer` to the PER_JVM; I think PER_EACH (test method) isn't implemented in the JUnit 4 runners yet.
Probably I am missing something but I think, I do not have the whole process or related use-cases in mind ;) - Maybe you , @rmannibucau , can add some more ideas / thoughts on the `mode` idea?
EDIT-1: I added some code to draft my idea / thoughts related to `modes`. Might be better to understand from the code point of view than from my words, if I got the idea correctly, so I added the related commits here.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tomee] rzo1 commented on a change in pull request #767: TOMEE-2977 - Provide a ApplicationComposer JUnit 5 Extension
Posted by GitBox <gi...@apache.org>.
rzo1 commented on a change in pull request #767:
URL: https://github.com/apache/tomee/pull/767#discussion_r591543473
##########
File path: container/openejb-core/src/main/java/org/apache/openejb/junit/jupiter/ApplicationComposerExtension.java
##########
@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit.jupiter;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.apache.openejb.testing.ApplicationComposers;
+import org.junit.jupiter.api.extension.*;
+
+public class ApplicationComposerExtension implements BeforeEachCallback, AfterEachCallback {
Review comment:
I made some progress based on your feedback, but I think it is still heavily **WIP** ;) It should now honor the test instance lifecycle. The `mode` thing is missing yet. I have to think about the impl for the proposed `mode` for controlling the container instances created during tests.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tomee] rzo1 commented on a change in pull request #767: TOMEE-2977 - Provide a ApplicationComposer JUnit 5 Extension
Posted by GitBox <gi...@apache.org>.
rzo1 commented on a change in pull request #767:
URL: https://github.com/apache/tomee/pull/767#discussion_r595909860
##########
File path: container/openejb-core/src/main/java/org/apache/openejb/junit/jupiter/ApplicationComposerExtension.java
##########
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit.jupiter;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.apache.openejb.testing.ApplicationComposers;
+import org.apache.openejb.testing.SingleApplicationComposerBase;
+import org.junit.jupiter.api.extension.*;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+
+public class ApplicationComposerExtension extends ApplicationComposerExtensionBase implements BeforeAllCallback, AfterAllCallback, BeforeEachCallback, AfterEachCallback {
+
+ private static final ExtensionContext.Namespace NAMESPACE = ExtensionContext.Namespace.create(ApplicationComposerExtension.class.getName());
+ private static final SingleApplicationComposerBase BASE = new SingleApplicationComposerBase();
+
+ @Override
+ public void beforeAll(ExtensionContext context) throws Exception {
+
+ if (isPerJvm(context)) {
Review comment:
I updated the related code fragments (via `validate(context)`. I don't know exactly, if I got the idea. Can you check, @rmannibucau ?
##########
File path: container/openejb-core/src/main/java/org/apache/openejb/junit/jupiter/ApplicationComposerExtension.java
##########
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit.jupiter;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.apache.openejb.testing.ApplicationComposers;
+import org.apache.openejb.testing.SingleApplicationComposerBase;
+import org.junit.jupiter.api.extension.*;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+
+public class ApplicationComposerExtension extends ApplicationComposerExtensionBase implements BeforeAllCallback, AfterAllCallback, BeforeEachCallback, AfterEachCallback {
+
+ private static final ExtensionContext.Namespace NAMESPACE = ExtensionContext.Namespace.create(ApplicationComposerExtension.class.getName());
+ private static final SingleApplicationComposerBase BASE = new SingleApplicationComposerBase();
+
+ @Override
+ public void beforeAll(ExtensionContext context) throws Exception {
+
+ if (isPerJvm(context)) {
+ BASE.start(context.getTestClass().orElse(null));
+ } else if (isPerAll(context)) {
+ doStart(context);
+ } else if (isPerDefault(context)) {
+ if (isPerClass(context)) {
+ doStart(context);
+ }
+ }
+
+ if (isPerClass(context)) {
+ doInject(context);
+ }
+ }
+
+ @Override
+ public void afterAll(ExtensionContext context) throws Exception {
+ if (isPerJvm(context) || isPerAll(context)) {
+ doRelease(context);
+ } else if (isPerDefault(context) && isPerClass(context)) {
+ doRelease(context);
+ }
+ }
+
+
+ @Override
+ public void beforeEach(ExtensionContext context) {
+
+ if (isPerEach(context)) {
Review comment:
Thx for clarification. Makes sense to me. I have updated the related code: https://github.com/apache/tomee/pull/767/commits/fc58a622b9f6aa8aa9ae6d69f91bcef11a4c11cc
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tomee] rmannibucau commented on a change in pull request #767: TOMEE-2977 - Provide a ApplicationComposer JUnit 5 Extension
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on a change in pull request #767:
URL: https://github.com/apache/tomee/pull/767#discussion_r595920794
##########
File path: container/openejb-core/src/main/java/org/apache/openejb/junit/jupiter/ApplicationComposerExtension.java
##########
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit.jupiter;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.apache.openejb.testing.ApplicationComposers;
+import org.apache.openejb.testing.SingleApplicationComposerBase;
+import org.junit.jupiter.api.extension.*;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+
+public class ApplicationComposerExtension extends ApplicationComposerExtensionBase implements BeforeAllCallback, AfterAllCallback, BeforeEachCallback, AfterEachCallback {
+
+ private static final ExtensionContext.Namespace NAMESPACE = ExtensionContext.Namespace.create(ApplicationComposerExtension.class.getName());
+ private static final SingleApplicationComposerBase BASE = new SingleApplicationComposerBase();
+
+ @Override
+ public void beforeAll(ExtensionContext context) throws Exception {
+
+ if (isPerJvm(context)) {
+ BASE.start(context.getTestClass().orElse(null));
+ } else if (isPerAll(context)) {
+ doStart(context);
+ } else if (isPerDefault(context)) {
+ if (isPerClass(context)) {
+ doStart(context);
+ }
+ }
+
+ if (isPerClass(context)) {
+ doInject(context);
+ }
+ }
+
+ @Override
+ public void afterAll(ExtensionContext context) throws Exception {
+ if (isPerJvm(context) || isPerAll(context)) {
+ doRelease(context);
+ } else if (isPerDefault(context) && isPerClass(context)) {
+ doRelease(context);
+ }
+ }
+
+
+ @Override
+ public void beforeEach(ExtensionContext context) {
+
+ if (isPerEach(context)) {
Review comment:
Yes, my starting point is that it is very error prone to test multiple times the same thing (let's ignore perf since it is simple tests). Since any "start" logic will put something (likely different) in the store, the presence can be used to test which case you are in to handle > start callbacks.
One way to see it is to have 3 impls (3 extensions) of BeforeAll+BeforeEach+AfterAll+AfterEach and put the impl in beforeAll of this extension which will then acts as a facade and select which impl to use in before all.
This way code path is unique each time and simple, only this extension beforeAll has some mode usage, all others are passthrough.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tomee] rzo1 commented on a change in pull request #767: TOMEE-2977 - Provide a ApplicationComposer JUnit 5 Extension
Posted by GitBox <gi...@apache.org>.
rzo1 commented on a change in pull request #767:
URL: https://github.com/apache/tomee/pull/767#discussion_r596170235
##########
File path: container/openejb-core/src/main/java/org/apache/openejb/junit/jupiter/ApplicationComposerExtension.java
##########
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit.jupiter;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.apache.openejb.testing.ApplicationComposers;
+import org.apache.openejb.testing.SingleApplicationComposerBase;
+import org.junit.jupiter.api.extension.*;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+
+public class ApplicationComposerExtension extends ApplicationComposerExtensionBase implements BeforeAllCallback, AfterAllCallback, BeforeEachCallback, AfterEachCallback {
+
+ private static final ExtensionContext.Namespace NAMESPACE = ExtensionContext.Namespace.create(ApplicationComposerExtension.class.getName());
+ private static final SingleApplicationComposerBase BASE = new SingleApplicationComposerBase();
+
+ @Override
+ public void beforeAll(ExtensionContext context) throws Exception {
+
+ if (isPerJvm(context)) {
+ BASE.start(context.getTestClass().orElse(null));
Review comment:
If I get it right, the PER_JVM case takes a fully assembled `@Application` defined in a class specified via `tomee.application-composer.application`. The rational is now to check for additional annotations (modules, classes, ...), which would lead to undefined behaviour. So it requires a check / validation for the following annotations
```
org.apache.openejb.testing.Component; //??
org.apache.openejb.testing.Module;
org.apache.openejb.testing.Classes;
org.apache.openejb.testing.Default //??
org.apache.openejb.testing.Jars; // ??
//similar @Deprecated annotated annotations in *.junit.*
```
on the test class and its related methods? Anything additional, @rmannibucau ?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tomee] rzo1 commented on pull request #767: TOMEE-2977 - Provide a ApplicationComposer JUnit 5 Extension
Posted by GitBox <gi...@apache.org>.
rzo1 commented on pull request #767:
URL: https://github.com/apache/tomee/pull/767#issuecomment-801386064
Thanks for the review, the feedback and the comments, @rmannibucau. If you don't mind, it might be ready for a 2nd review round ... ;)
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tomee] rzo1 commented on a change in pull request #767: TOMEE-2977 - Provide a ApplicationComposer JUnit 5 Extension
Posted by GitBox <gi...@apache.org>.
rzo1 commented on a change in pull request #767:
URL: https://github.com/apache/tomee/pull/767#discussion_r595907948
##########
File path: container/openejb-core/src/main/java/org/apache/openejb/junit/jupiter/ApplicationComposerExtension.java
##########
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit.jupiter;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.apache.openejb.testing.ApplicationComposers;
+import org.apache.openejb.testing.SingleApplicationComposerBase;
+import org.junit.jupiter.api.extension.*;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+
+public class ApplicationComposerExtension extends ApplicationComposerExtensionBase implements BeforeAllCallback, AfterAllCallback, BeforeEachCallback, AfterEachCallback {
+
+ private static final ExtensionContext.Namespace NAMESPACE = ExtensionContext.Namespace.create(ApplicationComposerExtension.class.getName());
+ private static final SingleApplicationComposerBase BASE = new SingleApplicationComposerBase();
+
+ @Override
+ public void beforeAll(ExtensionContext context) throws Exception {
+
+ if (isPerJvm(context)) {
+ BASE.start(context.getTestClass().orElse(null));
Review comment:
Couldn't find any documentation related to `@Application` at all. It seems, it isn't quite often used in the code base nor in the examples?
`ApplicationComposers#validate()`will complain and fail to start any container, if there is no `@Classes` or `@Default` annotation present?
Probably, I don't get the point here?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tomee] rzo1 commented on a change in pull request #767: TOMEE-2977 - Provide a ApplicationComposer JUnit 5 Extension
Posted by GitBox <gi...@apache.org>.
rzo1 commented on a change in pull request #767:
URL: https://github.com/apache/tomee/pull/767#discussion_r596842394
##########
File path: container/openejb-junit5/src/main/java/org/apache/openejb/junit5/ApplicationComposerPerJVMExtension.java
##########
@@ -0,0 +1,109 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit5;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.apache.openejb.testing.Classes;
+import org.apache.openejb.testing.Component;
+import org.apache.openejb.testing.Default;
+import org.apache.openejb.testing.Jars;
+import org.apache.openejb.testing.Module;
+import org.apache.openejb.testing.SingleApplicationComposerBase;
+import org.apache.xbean.finder.ClassFinder;
+import org.junit.jupiter.api.extension.AfterAllCallback;
+import org.junit.jupiter.api.extension.AfterEachCallback;
+import org.junit.jupiter.api.extension.BeforeAllCallback;
+import org.junit.jupiter.api.extension.BeforeEachCallback;
+import org.junit.jupiter.api.extension.ExtensionContext;
+import org.junit.jupiter.api.extension.TestInstances;
+
+import java.lang.annotation.Annotation;
+import java.util.ArrayList;
+import java.util.List;
+
+import static org.apache.openejb.util.Classes.ancestors;
+
+public class ApplicationComposerPerJVMExtension extends ApplicationComposerPerXYExtensionBase implements BeforeAllCallback, AfterAllCallback, BeforeEachCallback, AfterEachCallback {
+
+ private static final SingleApplicationComposerBase BASE = new SingleApplicationComposerBase();
+
+ @Override
+ void validate(ExtensionContext context) {
+ if (!isPerJvm(context) && BASE.isStarted()) {
+ //XXX: Future work: We might get it to work via a JVM singleton/lock, see https://github.com/apache/tomee/pull/767#discussion_r595343572
+ throw new OpenEJBRuntimeException("Cannot run PER_JVM in combination with PER_ALL, PER_EACH or AUTO");
+ }
+
+ Class<?> clazz = context.getTestClass()
+ .orElseThrow(() -> new OpenEJBRuntimeException("Could not obtain test class from extension context"));
+
+ final List<Throwable> errors = new ArrayList<>();
+
+ ClassFinder classFinder = new ClassFinder(ancestors(clazz));
+
+ Class<? extends Annotation>[] toCheck = new Class[]{Component.class, Module.class, Classes.class, Default.class, Jars.class};
+
+ for (Class<? extends Annotation> annotation : toCheck) {
+ if (classFinder.isAnnotationPresent(annotation)) {
+ errors.add(new Exception("@" + annotation.getName() + " is not allowed with @Application in PER_JVM mode"));
+ }
+ }
+
+ if (!errors.isEmpty()) {
+ throw new OpenEJBRuntimeException(errors.toString());
+ }
+ }
+
+ @Override
+ public void beforeAll(ExtensionContext context) throws Exception {
+ super.beforeAll(context);
+
+ BASE.start(context.getTestClass().orElse(null));
+ if (isPerClassLifecycle(context)) {
+ doInject(context);
+ }
+ }
+
+ @Override
+ public void beforeEach(ExtensionContext context) throws Exception {
+ if (isPerMethodLifecycle(context)) {
+ doInject(context);
+ }
+ }
+
+ private void doInject(final ExtensionContext extensionContext) {
Review comment:
Think it is addressed now.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tomee] rzo1 commented on a change in pull request #767: TOMEE-2977 - Provide a ApplicationComposer JUnit 5 Extension
Posted by GitBox <gi...@apache.org>.
rzo1 commented on a change in pull request #767:
URL: https://github.com/apache/tomee/pull/767#discussion_r590433216
##########
File path: container/openejb-core/src/main/java/org/apache/openejb/junit/jupiter/ApplicationComposerExtension.java
##########
@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit.jupiter;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.apache.openejb.testing.ApplicationComposers;
+import org.junit.jupiter.api.extension.*;
+
+public class ApplicationComposerExtension implements BeforeEachCallback, AfterEachCallback {
Review comment:
Thx. I updated things related to 2. and 3.
I have to think about 1. (because I think, that I didnt get it) and read / study the documentation related to `DynamicTests` in JUnit 5. Do you have an example of "splitting test definitions into multiple classes"?
Might get you wrong, but did you refer to `Test Instance Lifecycles` when talking about `@ApplicationComposer(mode = [PER_JVM|PER_ALL|PER_EACH])` ?
So we get smth like this, if the mode is specified?
- PER_JVM --> `SingleApplicationComposerExtension` (like TomEE embedded?)
- PER_ALL --> `@TestInstance(Lifecycle.PER_CLASS)` // new test instance will be created once per test class.
- PER_EACH --> `@TestInstance(Lifecycle.PER_METHOD)` // a new test instance will be created for each test method, test factory method, or test template method (default in JUnit 4)
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tomee] rmannibucau commented on a change in pull request #767: TOMEE-2977 - Provide a ApplicationComposer JUnit 5 Extension
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on a change in pull request #767:
URL: https://github.com/apache/tomee/pull/767#discussion_r596599296
##########
File path: container/openejb-junit5/pom.xml
##########
@@ -0,0 +1,90 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <parent>
+ <artifactId>container</artifactId>
+ <groupId>org.apache.tomee</groupId>
+ <version>8.0.7-SNAPSHOT</version>
+ </parent>
+ <modelVersion>4.0.0</modelVersion>
+
+ <artifactId>openejb-junit5</artifactId>
+ <packaging>jar</packaging>
+ <name>TomEE :: Container :: JUnit 5</name>
+
+ <properties>
+ <tomee.build.name>${project.groupId}.container.junit5</tomee.build.name>
+ <netbeans.hint.license>openejb</netbeans.hint.license>
+ <jacocoArgLine/>
+ </properties>
+
+ <dependencies>
+ <dependency>
+ <groupId>${project.groupId}</groupId>
+ <artifactId>openejb-core</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.junit.jupiter</groupId>
+ <artifactId>junit-jupiter-api</artifactId>
+ <scope>provided</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.junit.jupiter</groupId>
+ <artifactId>junit-jupiter-engine</artifactId>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.openejb.itests</groupId>
+ <artifactId>failover-ejb</artifactId>
+ <version>${project.version}</version>
+ <scope>test</scope>
+ </dependency>
+ </dependencies>
+
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-surefire-plugin</artifactId>
+ <version>${surefire.junit5.version}</version>
+ <configuration>
+ <childDelegation>true</childDelegation>
+ <reuseForks>false</reuseForks>
+ <forkCount>1</forkCount>
+ <argLine>
+ ${jacocoArgLine}
+ -enableassertions
+ </argLine>
+ <workingDirectory>${project.basedir}/target</workingDirectory>
+ <systemPropertyVariables>
+ <openejb.home>${project.basedir}/target/test-classes</openejb.home>
+ </systemPropertyVariables>
+ <excludes>
Review comment:
an excluded test is a test to delete ;)
joke apart, use N executions to ensure all tests run in the build.
##########
File path: container/openejb-junit5/src/main/java/org/apache/openejb/junit5/ApplicationComposerExtensionBase.java
##########
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit5;
+
+import org.junit.jupiter.api.TestInstance;
+import org.junit.jupiter.api.extension.ExtensionContext;
+import org.junit.platform.commons.util.AnnotationUtils;
+
+import java.util.Optional;
+
+public abstract class ApplicationComposerExtensionBase {
+
+ boolean isPerClassLifecycle(final ExtensionContext context) {
+ return isPerTestInstanceLifecycle(context, TestInstance.Lifecycle.PER_CLASS);
+ }
+
+ boolean isPerMethodLifecycle(final ExtensionContext context) {
+ return isPerTestInstanceLifecycle(context, TestInstance.Lifecycle.PER_METHOD);
+ }
+
+ boolean isPerTestInstanceLifecycle(final ExtensionContext context, TestInstance.Lifecycle lifecycle) {
+ return context.getTestInstanceLifecycle()
+ .map(it -> it.equals(lifecycle))
+ .orElse(false);
+ }
+
+ protected boolean isPerEach(final ExtensionContext context) {
+ return checkMode(context, ExtensionMode.PER_EACH);
+ }
+
+ boolean isPerAll(final ExtensionContext context) {
+ return checkMode(context, ExtensionMode.PER_ALL);
+ }
+
+ boolean isPerJvm(final ExtensionContext context) {
+ return checkMode(context, ExtensionMode.PER_JVM);
+ }
+
+ boolean isPerDefault(final ExtensionContext context) {
+ return checkMode(context, ExtensionMode.AUTO);
+ }
+
+ boolean checkMode(final ExtensionContext context, ExtensionMode extensionMode ) {
+ return extensionMode == getModeFromAnnotation(context);
+ }
+
+ ExtensionMode getModeFromAnnotation(final ExtensionContext context) {
+ if (context.getTestClass().isPresent()) {
Review comment:
```
return context.getTestClass()
.flatMap(test -> AnnotationUtils.findAnnotation(test, RunWithApplicationComposer.class))
.map(RunWithApplicationComposer::mode)
.orElse(ExtensionMode.AUTO);
```
Overall optional.get() should almost never be used ;)
##########
File path: container/openejb-junit5/src/main/java/org/apache/openejb/junit5/ApplicationComposerPerXYExtensionBase.java
##########
@@ -0,0 +1,109 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit5;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.apache.openejb.testing.ApplicationComposers;
+import org.junit.jupiter.api.extension.AfterAllCallback;
+import org.junit.jupiter.api.extension.AfterEachCallback;
+import org.junit.jupiter.api.extension.BeforeAllCallback;
+import org.junit.jupiter.api.extension.BeforeEachCallback;
+import org.junit.jupiter.api.extension.ExtensionContext;
+import org.junit.jupiter.api.extension.TestInstances;
+
+import java.util.List;
+
+abstract class ApplicationComposerPerXYExtensionBase extends ApplicationComposerExtensionBase implements BeforeAllCallback, AfterAllCallback, BeforeEachCallback, AfterEachCallback {
+
+ private static final ExtensionContext.Namespace NAMESPACE = ExtensionContext.Namespace.create(ApplicationComposerPerXYExtensionBase.class.getName());
+
+ private final Object[] modules;
+
+ public ApplicationComposerPerXYExtensionBase() {
+ this((Object[]) null);
+ }
+
+ public ApplicationComposerPerXYExtensionBase(Object... modules) {
+ this.modules = modules;
+ }
+
+ abstract void validate(ExtensionContext context);
Review comment:
I'd put it protected and not package scope, wdyt?
##########
File path: container/openejb-junit5/src/main/java/org/apache/openejb/junit5/ApplicationComposerPerAllExtension.java
##########
@@ -0,0 +1,51 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit5;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.junit.jupiter.api.extension.AfterAllCallback;
+import org.junit.jupiter.api.extension.AfterEachCallback;
+import org.junit.jupiter.api.extension.BeforeAllCallback;
+import org.junit.jupiter.api.extension.BeforeEachCallback;
+import org.junit.jupiter.api.extension.ExtensionContext;
+
+public class ApplicationComposerPerAllExtension extends ApplicationComposerPerXYExtensionBase implements BeforeAllCallback, AfterAllCallback, BeforeEachCallback, AfterEachCallback {
+
+ public ApplicationComposerPerAllExtension() {
+ this((Object[]) null);
+ }
+
+ public ApplicationComposerPerAllExtension(Object... modules) {
+ super(modules);
+ }
+
+ @Override
+ void validate(ExtensionContext context) {
+ if (isPerAll(context) && isPerMethodLifecycle(context)) {
+ //XXX: Might be possible to make this work - for now, we are going to throw something...
+ throw new OpenEJBRuntimeException("Cannot run PER_ALL in combination with TestInstance.Lifecycle.PER_METHOD.");
Review comment:
maybe just log a warning/multiline info log, it means the injections will be lost at test > 1 but for 1 test it is fine and for N > 1 tests it works if you use CDI.current() or InitialContext or pure client (http) to impl the test.
##########
File path: container/openejb-junit5/src/main/java/org/apache/openejb/junit5/ApplicationComposerExtension.java
##########
@@ -0,0 +1,87 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit5;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.junit.jupiter.api.extension.AfterAllCallback;
+import org.junit.jupiter.api.extension.AfterEachCallback;
+import org.junit.jupiter.api.extension.BeforeAllCallback;
+import org.junit.jupiter.api.extension.BeforeEachCallback;
+import org.junit.jupiter.api.extension.ExtensionContext;
+
+public class ApplicationComposerExtension extends ApplicationComposerExtensionBase implements BeforeAllCallback, AfterAllCallback, BeforeEachCallback, AfterEachCallback {
+
+ private static final ExtensionContext.Namespace NAMESPACE = ExtensionContext.Namespace.create(ApplicationComposerExtension.class.getName());
+
+ private final Object[] modules;
+
+ public ApplicationComposerExtension() {
+ this((Object[]) null);
+ }
+
+ public ApplicationComposerExtension(Object... modules) {
+ this.modules = modules;
+ }
+
+ @Override
+ public void beforeAll(ExtensionContext context) throws Exception {
+
+ if (!isPerJvm(context) && ApplicationComposerPerJVMExtension.isStarted()) {
+ //XXX: Future work: We might get it to work via a JVM singleton/lock, see https://github.com/apache/tomee/pull/767#discussion_r595343572
+ throw new OpenEJBRuntimeException("Cannot run PER_JVM in combination with PER_ALL, PER_EACH or AUTO");
+ }
+
+ if (isPerJvm(context)) {
+ context.getStore(NAMESPACE).put(ApplicationComposerPerXYExtensionBase.class, new ApplicationComposerPerJVMExtension());
Review comment:
if modules.length > 0 => throw it is unsupported, use -Dtomee.application etc
##########
File path: container/openejb-junit5/src/main/java/org/apache/openejb/junit5/AfterReleaserBase.java
##########
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit5;
+
+import org.apache.openejb.testing.ApplicationComposers;
+import org.junit.jupiter.api.extension.ExtensionContext;
+
+public abstract class AfterReleaserBase extends ApplicationComposerExtensionBase {
+
+ private final ExtensionContext.Namespace namespace;
+
+ AfterReleaserBase(ExtensionContext.Namespace namespace) {
+ this.namespace = namespace;
+ }
+
+ void run(final ExtensionContext extensionContext) throws Exception {
+ doRelease(extensionContext);
+ }
+
+ void doRelease(final ExtensionContext extensionContext) throws Exception {
Review comment:
shouldn't it be merged with run? don't think it is needed to split anymore, wdyt?
##########
File path: container/openejb-junit5/src/main/java/org/apache/openejb/junit5/ApplicationComposerExtension.java
##########
@@ -0,0 +1,87 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit5;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.junit.jupiter.api.extension.AfterAllCallback;
+import org.junit.jupiter.api.extension.AfterEachCallback;
+import org.junit.jupiter.api.extension.BeforeAllCallback;
+import org.junit.jupiter.api.extension.BeforeEachCallback;
+import org.junit.jupiter.api.extension.ExtensionContext;
+
+public class ApplicationComposerExtension extends ApplicationComposerExtensionBase implements BeforeAllCallback, AfterAllCallback, BeforeEachCallback, AfterEachCallback {
+
+ private static final ExtensionContext.Namespace NAMESPACE = ExtensionContext.Namespace.create(ApplicationComposerExtension.class.getName());
+
+ private final Object[] modules;
+
+ public ApplicationComposerExtension() {
+ this((Object[]) null);
+ }
+
+ public ApplicationComposerExtension(Object... modules) {
+ this.modules = modules;
+ }
+
+ @Override
+ public void beforeAll(ExtensionContext context) throws Exception {
+
+ if (!isPerJvm(context) && ApplicationComposerPerJVMExtension.isStarted()) {
Review comment:
let the delegate validate it in before all? the trick will be to make the validation work when this facade is not used too so each impl must validate the combinations (or the parent do it all) so the facade does not need any validation I think
##########
File path: container/openejb-junit5/src/main/java/org/apache/openejb/junit5/ApplicationComposerPerJVMExtension.java
##########
@@ -0,0 +1,109 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit5;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.apache.openejb.testing.Classes;
+import org.apache.openejb.testing.Component;
+import org.apache.openejb.testing.Default;
+import org.apache.openejb.testing.Jars;
+import org.apache.openejb.testing.Module;
+import org.apache.openejb.testing.SingleApplicationComposerBase;
+import org.apache.xbean.finder.ClassFinder;
+import org.junit.jupiter.api.extension.AfterAllCallback;
+import org.junit.jupiter.api.extension.AfterEachCallback;
+import org.junit.jupiter.api.extension.BeforeAllCallback;
+import org.junit.jupiter.api.extension.BeforeEachCallback;
+import org.junit.jupiter.api.extension.ExtensionContext;
+import org.junit.jupiter.api.extension.TestInstances;
+
+import java.lang.annotation.Annotation;
+import java.util.ArrayList;
+import java.util.List;
+
+import static org.apache.openejb.util.Classes.ancestors;
+
+public class ApplicationComposerPerJVMExtension extends ApplicationComposerPerXYExtensionBase implements BeforeAllCallback, AfterAllCallback, BeforeEachCallback, AfterEachCallback {
+
+ private static final SingleApplicationComposerBase BASE = new SingleApplicationComposerBase();
+
+ @Override
+ void validate(ExtensionContext context) {
+ if (!isPerJvm(context) && BASE.isStarted()) {
+ //XXX: Future work: We might get it to work via a JVM singleton/lock, see https://github.com/apache/tomee/pull/767#discussion_r595343572
+ throw new OpenEJBRuntimeException("Cannot run PER_JVM in combination with PER_ALL, PER_EACH or AUTO");
+ }
+
+ Class<?> clazz = context.getTestClass()
+ .orElseThrow(() -> new OpenEJBRuntimeException("Could not obtain test class from extension context"));
+
+ final List<Throwable> errors = new ArrayList<>();
+
+ ClassFinder classFinder = new ClassFinder(ancestors(clazz));
+
+ Class<? extends Annotation>[] toCheck = new Class[]{Component.class, Module.class, Classes.class, Default.class, Jars.class};
+
+ for (Class<? extends Annotation> annotation : toCheck) {
+ if (classFinder.isAnnotationPresent(annotation)) {
+ errors.add(new Exception("@" + annotation.getName() + " is not allowed with @Application in PER_JVM mode"));
+ }
+ }
+
+ if (!errors.isEmpty()) {
+ throw new OpenEJBRuntimeException(errors.toString());
+ }
+ }
+
+ @Override
+ public void beforeAll(ExtensionContext context) throws Exception {
+ super.beforeAll(context);
+
+ BASE.start(context.getTestClass().orElse(null));
+ if (isPerClassLifecycle(context)) {
+ doInject(context);
+ }
+ }
+
+ @Override
+ public void beforeEach(ExtensionContext context) throws Exception {
+ if (isPerMethodLifecycle(context)) {
+ doInject(context);
+ }
+ }
+
+ private void doInject(final ExtensionContext extensionContext) {
Review comment:
this one should be the same for all cases, even global one, no?
##########
File path: container/openejb-junit5/src/main/java/org/apache/openejb/junit5/ApplicationComposerPerEachExtension.java
##########
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit5;
+
+import org.junit.jupiter.api.extension.AfterAllCallback;
+import org.junit.jupiter.api.extension.AfterEachCallback;
+import org.junit.jupiter.api.extension.BeforeAllCallback;
+import org.junit.jupiter.api.extension.BeforeEachCallback;
+import org.junit.jupiter.api.extension.ExtensionContext;
+
+public class ApplicationComposerPerEachExtension extends ApplicationComposerPerXYExtensionBase implements BeforeAllCallback, AfterAllCallback, BeforeEachCallback, AfterEachCallback {
+
+ public ApplicationComposerPerEachExtension() {
+ this((Object[]) null);
+ }
+
+ public ApplicationComposerPerEachExtension(Object... modules) {
+ super(modules);
+ }
+
+ @Override
+ void validate(ExtensionContext context) {
+ //no-op
+ }
+
+ @Override
+ public void afterAll(ExtensionContext context) throws Exception {
+ addAfterEachReleaser(context);
Review comment:
this one looks fishy, shouldnt it be in beforeEach?
##########
File path: container/openejb-junit5/pom.xml
##########
@@ -0,0 +1,90 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <parent>
+ <artifactId>container</artifactId>
+ <groupId>org.apache.tomee</groupId>
+ <version>8.0.7-SNAPSHOT</version>
+ </parent>
+ <modelVersion>4.0.0</modelVersion>
+
+ <artifactId>openejb-junit5</artifactId>
+ <packaging>jar</packaging>
+ <name>TomEE :: Container :: JUnit 5</name>
+
+ <properties>
+ <tomee.build.name>${project.groupId}.container.junit5</tomee.build.name>
+ <netbeans.hint.license>openejb</netbeans.hint.license>
+ <jacocoArgLine/>
+ </properties>
+
+ <dependencies>
+ <dependency>
+ <groupId>${project.groupId}</groupId>
+ <artifactId>openejb-core</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.junit.jupiter</groupId>
+ <artifactId>junit-jupiter-api</artifactId>
+ <scope>provided</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.junit.jupiter</groupId>
+ <artifactId>junit-jupiter-engine</artifactId>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.openejb.itests</groupId>
+ <artifactId>failover-ejb</artifactId>
+ <version>${project.version}</version>
+ <scope>test</scope>
+ </dependency>
+ </dependencies>
+
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-surefire-plugin</artifactId>
+ <version>${surefire.junit5.version}</version>
Review comment:
we should have a single surefire version for the full build IMHO
##########
File path: container/openejb-junit5/src/main/java/org/apache/openejb/junit5/ApplicationComposerPerDefaultExtension.java
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit5;
+
+import org.junit.jupiter.api.extension.AfterAllCallback;
+import org.junit.jupiter.api.extension.AfterEachCallback;
+import org.junit.jupiter.api.extension.BeforeAllCallback;
+import org.junit.jupiter.api.extension.BeforeEachCallback;
+import org.junit.jupiter.api.extension.ExtensionContext;
+
+public class ApplicationComposerPerDefaultExtension extends ApplicationComposerPerXYExtensionBase implements BeforeAllCallback, AfterAllCallback, BeforeEachCallback, AfterEachCallback {
+
+ public ApplicationComposerPerDefaultExtension() {
+ this((Object[]) null);
+ }
+
+ public ApplicationComposerPerDefaultExtension(Object... modules) {
+ super(modules);
+ }
+
+ @Override
+ void validate(ExtensionContext context) {
Review comment:
I suspect parent one can be empty by defaul tinstead of being abstract
##########
File path: examples/junit5-application-composer/pom.xml
##########
@@ -0,0 +1,120 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+<!-- $Rev: 636494 $ $Date: 2008-03-12 21:24:02 +0100 (Wed, 12 Mar 2008) $ -->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+ <groupId>org.superbiz</groupId>
+ <artifactId>junit5-application-composer</artifactId>
+ <packaging>jar</packaging>
+ <version>8.0.7-SNAPSHOT</version>
+ <name>TomEE :: Examples :: JUnit 5 :: Application Composer</name>
+ <properties>
+ <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+ </properties>
+ <build>
+ <defaultGoal>install</defaultGoal>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-compiler-plugin</artifactId>
+ <version>3.5.1</version>
+ <configuration>
+ <source>1.8</source>
+ <target>1.8</target>
+ </configuration>
+ </plugin>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-surefire-plugin</artifactId>
+ <version>3.0.0-M5</version>
+ <configuration>
+ <argLine>-Djdk.attach.allowAttachSelf</argLine>
+ </configuration>
+ </plugin>
+ <plugin>
Review comment:
this one is not needed since the deployment is skipped by putting it locally IMHO
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tomee] rmannibucau commented on a change in pull request #767: TOMEE-2977 - Provide a ApplicationComposer JUnit 5 Extension
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on a change in pull request #767:
URL: https://github.com/apache/tomee/pull/767#discussion_r595289263
##########
File path: container/openejb-core/src/main/java/org/apache/openejb/junit/jupiter/ApplicationComposerExtensionBase.java
##########
@@ -26,4 +27,36 @@ boolean isPerClass(final ExtensionContext context) {
.map(it -> it.equals(TestInstance.Lifecycle.PER_CLASS))
.orElse(false);
}
+
+ boolean isPerEach(final ExtensionContext context) {
+ return checkMode(context, ExtensionMode.PER_EACH);
+ }
+
+ boolean isPerAll(final ExtensionContext context) {
+ return checkMode(context, ExtensionMode.PER_ALL);
+ }
+
+ boolean isPerJvm(final ExtensionContext context) {
+ return checkMode(context, ExtensionMode.PER_JVM);
+ }
+
+ boolean isPerDefault(final ExtensionContext context) {
+ return checkMode(context, ExtensionMode.AUTO);
+ }
+
+ boolean checkMode(final ExtensionContext context, ExtensionMode extensionMode ) {
+ return extensionMode == getModeFromAnnotation(context);
+ }
+
+ ExtensionMode getModeFromAnnotation(final ExtensionContext context) {
+ if (context.getTestClass().isPresent()) {
+
+ RunWithApplicationComposer a = context.getTestClass().get().getAnnotation(RunWithApplicationComposer.class);
Review comment:
AnnotationUtils.findAnnotation ;) (will support junit5 stereotypes otherwise you loose it by pure reflection)
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tomee] rzo1 commented on a change in pull request #767: TOMEE-2977 - Provide a ApplicationComposer JUnit 5 Extension
Posted by GitBox <gi...@apache.org>.
rzo1 commented on a change in pull request #767:
URL: https://github.com/apache/tomee/pull/767#discussion_r590638509
##########
File path: container/openejb-core/src/main/java/org/apache/openejb/junit/jupiter/ApplicationComposerExtension.java
##########
@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit.jupiter;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.apache.openejb.testing.ApplicationComposers;
+import org.junit.jupiter.api.extension.*;
+
+public class ApplicationComposerExtension implements BeforeEachCallback, AfterEachCallback {
Review comment:
Thanks for the details!
> The split of an app composer app in multiple class is actually what you use ;). The constructor of ApplicationComposers takes N objects, just pass all test classes if you want a single deployment for all tests and enable each test to deploy part of the app.
Afaik the extension context only provides the related test class. So it would be something like
- Get inner classes from the test class
- Create instances of these inner test classes
- Supply the test class as well as the N instances of the inner test classes to `ApplicationComposers` (like it is done in `AppComposerAsJUnitRuleWithReusableModulesTest` via JUnit 4 rules) ?
> About the lifecycle it is not 1-1 with test instance lifecycle but the same idea for the container lifecycle (but you can have one test instance per test and one container instance for all tests of a single class when you want to reset another extension data/field instances for example). We can align on the default if nothing is set (so add to the enum an AUTO mode ;)) but must be configurable.
Thanks! I think, I am now getting a better understanding of the idea. I also checked the JUnit 5 extension impl of meecrowave, which was really helpful.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tomee] rzo1 edited a comment on pull request #767: TOMEE-2977 - Provide a ApplicationComposer JUnit 5 Extension
Posted by GitBox <gi...@apache.org>.
rzo1 edited a comment on pull request #767:
URL: https://github.com/apache/tomee/pull/767#issuecomment-801920909
Should now contain the proposed fixes / enhancements.
Tests are now executed in an additional phase. Let's wait if we receive some additional feedback.
EDIT-1: Rebased the PR - Jenkins PR builder was some sort of stuck. Hopefully it will now pick up the PR again.
--
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:
users@infra.apache.org
[GitHub] [tomee] rzo1 commented on a change in pull request #767: TOMEE-2977 - Provide a ApplicationComposer JUnit 5 Extension
Posted by GitBox <gi...@apache.org>.
rzo1 commented on a change in pull request #767:
URL: https://github.com/apache/tomee/pull/767#discussion_r595971489
##########
File path: container/openejb-core/src/main/java/org/apache/openejb/junit/jupiter/ApplicationComposerExtension.java
##########
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit.jupiter;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.apache.openejb.testing.ApplicationComposers;
+import org.apache.openejb.testing.SingleApplicationComposerBase;
+import org.junit.jupiter.api.extension.*;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+
+public class ApplicationComposerExtension extends ApplicationComposerExtensionBase implements BeforeAllCallback, AfterAllCallback, BeforeEachCallback, AfterEachCallback {
+
+ private static final ExtensionContext.Namespace NAMESPACE = ExtensionContext.Namespace.create(ApplicationComposerExtension.class.getName());
+ private static final SingleApplicationComposerBase BASE = new SingleApplicationComposerBase();
+
+ @Override
+ public void beforeAll(ExtensionContext context) throws Exception {
+
+ if (isPerJvm(context)) {
Review comment:
1.) -> might be addressed with https://github.com/apache/tomee/pull/767/commits/6fba175b05270b9838b87a98d346da246135c4d0
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tomee] rzo1 commented on a change in pull request #767: TOMEE-2977 - Provide a ApplicationComposer JUnit 5 Extension
Posted by GitBox <gi...@apache.org>.
rzo1 commented on a change in pull request #767:
URL: https://github.com/apache/tomee/pull/767#discussion_r592724306
##########
File path: container/openejb-core/src/main/java/org/apache/openejb/junit/jupiter/ApplicationComposerExtension.java
##########
@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit.jupiter;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.apache.openejb.testing.ApplicationComposers;
+import org.junit.jupiter.api.extension.*;
+
+public class ApplicationComposerExtension implements BeforeEachCallback, AfterEachCallback {
Review comment:
The current impl relies on the test's life cycle to start a container or perform related injections.
Atm the impl contains annotations and extensions for
- `RunWithSingleApplicationComposer`
- `RunWithApplicationComposer`
as well as a
- `TomEEEmbeddedExtension`
From my understanding, the proposed `mode` is solely responsible to configure how (or when) the underlying container is started? I.e. one container to run all tests within a single test class (PER_ALL), one container for each test method of one test class (PER_EACH) and one container to run multiple test classes (PER_JVM), right? Is this the same idea, which is currently conducted through different JUnit4 runners?
From my understanding of the current JUnit implementation, the `ApplicationComposer` seems to be something similar to the PER_ALL and the `EmbeddedTomEERunner` / `SingleApplicationComposer` to the PER_JVM; I think PER_EACH (test method) isn't implemented in the JUnit 4 runners yet.
Probably I am missing something but I think, I do not have the whole process or related use-cases in mind ;) - Maybe you , @rmannibucau , can add some more ideas / thoughts on the `mode` idea?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tomee] rmannibucau commented on a change in pull request #767: TOMEE-2977 - Provide a ApplicationComposer JUnit 5 Extension
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on a change in pull request #767:
URL: https://github.com/apache/tomee/pull/767#discussion_r596769746
##########
File path: container/openejb-junit5/src/main/java/org/apache/openejb/junit5/ApplicationComposerPerJVMExtension.java
##########
@@ -0,0 +1,109 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit5;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.apache.openejb.testing.Classes;
+import org.apache.openejb.testing.Component;
+import org.apache.openejb.testing.Default;
+import org.apache.openejb.testing.Jars;
+import org.apache.openejb.testing.Module;
+import org.apache.openejb.testing.SingleApplicationComposerBase;
+import org.apache.xbean.finder.ClassFinder;
+import org.junit.jupiter.api.extension.AfterAllCallback;
+import org.junit.jupiter.api.extension.AfterEachCallback;
+import org.junit.jupiter.api.extension.BeforeAllCallback;
+import org.junit.jupiter.api.extension.BeforeEachCallback;
+import org.junit.jupiter.api.extension.ExtensionContext;
+import org.junit.jupiter.api.extension.TestInstances;
+
+import java.lang.annotation.Annotation;
+import java.util.ArrayList;
+import java.util.List;
+
+import static org.apache.openejb.util.Classes.ancestors;
+
+public class ApplicationComposerPerJVMExtension extends ApplicationComposerPerXYExtensionBase implements BeforeAllCallback, AfterAllCallback, BeforeEachCallback, AfterEachCallback {
+
+ private static final SingleApplicationComposerBase BASE = new SingleApplicationComposerBase();
+
+ @Override
+ void validate(ExtensionContext context) {
+ if (!isPerJvm(context) && BASE.isStarted()) {
+ //XXX: Future work: We might get it to work via a JVM singleton/lock, see https://github.com/apache/tomee/pull/767#discussion_r595343572
+ throw new OpenEJBRuntimeException("Cannot run PER_JVM in combination with PER_ALL, PER_EACH or AUTO");
+ }
+
+ Class<?> clazz = context.getTestClass()
+ .orElseThrow(() -> new OpenEJBRuntimeException("Could not obtain test class from extension context"));
+
+ final List<Throwable> errors = new ArrayList<>();
+
+ ClassFinder classFinder = new ClassFinder(ancestors(clazz));
+
+ Class<? extends Annotation>[] toCheck = new Class[]{Component.class, Module.class, Classes.class, Default.class, Jars.class};
+
+ for (Class<? extends Annotation> annotation : toCheck) {
+ if (classFinder.isAnnotationPresent(annotation)) {
+ errors.add(new Exception("@" + annotation.getName() + " is not allowed with @Application in PER_JVM mode"));
+ }
+ }
+
+ if (!errors.isEmpty()) {
+ throw new OpenEJBRuntimeException(errors.toString());
+ }
+ }
+
+ @Override
+ public void beforeAll(ExtensionContext context) throws Exception {
+ super.beforeAll(context);
+
+ BASE.start(context.getTestClass().orElse(null));
+ if (isPerClassLifecycle(context)) {
+ doInject(context);
+ }
+ }
+
+ @Override
+ public void beforeEach(ExtensionContext context) throws Exception {
+ if (isPerMethodLifecycle(context)) {
+ doInject(context);
+ }
+ }
+
+ private void doInject(final ExtensionContext extensionContext) {
Review comment:
injection is the same in all cases (inject with the deployed app) so code path should be the same IMHO.
it is fine to make public the enrich method of the applicationcomposers instance and store in it the bean context to be able to inject later (probably expose enrich(Object) only).
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tomee] rzo1 commented on a change in pull request #767: TOMEE-2977 - Provide a ApplicationComposer JUnit 5 Extension
Posted by GitBox <gi...@apache.org>.
rzo1 commented on a change in pull request #767:
URL: https://github.com/apache/tomee/pull/767#discussion_r595909860
##########
File path: container/openejb-core/src/main/java/org/apache/openejb/junit/jupiter/ApplicationComposerExtension.java
##########
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit.jupiter;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.apache.openejb.testing.ApplicationComposers;
+import org.apache.openejb.testing.SingleApplicationComposerBase;
+import org.junit.jupiter.api.extension.*;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+
+public class ApplicationComposerExtension extends ApplicationComposerExtensionBase implements BeforeAllCallback, AfterAllCallback, BeforeEachCallback, AfterEachCallback {
+
+ private static final ExtensionContext.Namespace NAMESPACE = ExtensionContext.Namespace.create(ApplicationComposerExtension.class.getName());
+ private static final SingleApplicationComposerBase BASE = new SingleApplicationComposerBase();
+
+ @Override
+ public void beforeAll(ExtensionContext context) throws Exception {
+
+ if (isPerJvm(context)) {
Review comment:
I updated the related code fragments (via `validate(context)`. I don't know exactly, if I got the idea. Can you check, @rmannibucau ?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tomee] rzo1 commented on a change in pull request #767: TOMEE-2977 - Provide a ApplicationComposer JUnit 5 Extension
Posted by GitBox <gi...@apache.org>.
rzo1 commented on a change in pull request #767:
URL: https://github.com/apache/tomee/pull/767#discussion_r596740918
##########
File path: container/openejb-junit5/src/main/java/org/apache/openejb/junit5/ApplicationComposerPerJVMExtension.java
##########
@@ -0,0 +1,109 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit5;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.apache.openejb.testing.Classes;
+import org.apache.openejb.testing.Component;
+import org.apache.openejb.testing.Default;
+import org.apache.openejb.testing.Jars;
+import org.apache.openejb.testing.Module;
+import org.apache.openejb.testing.SingleApplicationComposerBase;
+import org.apache.xbean.finder.ClassFinder;
+import org.junit.jupiter.api.extension.AfterAllCallback;
+import org.junit.jupiter.api.extension.AfterEachCallback;
+import org.junit.jupiter.api.extension.BeforeAllCallback;
+import org.junit.jupiter.api.extension.BeforeEachCallback;
+import org.junit.jupiter.api.extension.ExtensionContext;
+import org.junit.jupiter.api.extension.TestInstances;
+
+import java.lang.annotation.Annotation;
+import java.util.ArrayList;
+import java.util.List;
+
+import static org.apache.openejb.util.Classes.ancestors;
+
+public class ApplicationComposerPerJVMExtension extends ApplicationComposerPerXYExtensionBase implements BeforeAllCallback, AfterAllCallback, BeforeEachCallback, AfterEachCallback {
+
+ private static final SingleApplicationComposerBase BASE = new SingleApplicationComposerBase();
+
+ @Override
+ void validate(ExtensionContext context) {
+ if (!isPerJvm(context) && BASE.isStarted()) {
+ //XXX: Future work: We might get it to work via a JVM singleton/lock, see https://github.com/apache/tomee/pull/767#discussion_r595343572
+ throw new OpenEJBRuntimeException("Cannot run PER_JVM in combination with PER_ALL, PER_EACH or AUTO");
+ }
+
+ Class<?> clazz = context.getTestClass()
+ .orElseThrow(() -> new OpenEJBRuntimeException("Could not obtain test class from extension context"));
+
+ final List<Throwable> errors = new ArrayList<>();
+
+ ClassFinder classFinder = new ClassFinder(ancestors(clazz));
+
+ Class<? extends Annotation>[] toCheck = new Class[]{Component.class, Module.class, Classes.class, Default.class, Jars.class};
+
+ for (Class<? extends Annotation> annotation : toCheck) {
+ if (classFinder.isAnnotationPresent(annotation)) {
+ errors.add(new Exception("@" + annotation.getName() + " is not allowed with @Application in PER_JVM mode"));
+ }
+ }
+
+ if (!errors.isEmpty()) {
+ throw new OpenEJBRuntimeException(errors.toString());
+ }
+ }
+
+ @Override
+ public void beforeAll(ExtensionContext context) throws Exception {
+ super.beforeAll(context);
+
+ BASE.start(context.getTestClass().orElse(null));
+ if (isPerClassLifecycle(context)) {
+ doInject(context);
+ }
+ }
+
+ @Override
+ public void beforeEach(ExtensionContext context) throws Exception {
+ if (isPerMethodLifecycle(context)) {
+ doInject(context);
+ }
+ }
+
+ private void doInject(final ExtensionContext extensionContext) {
Review comment:
From my understanding of the code:
`ApplicationComposers.before(...)` starts a container, deploys the app and does the injection internally (in `doStart(...)`) ? In the `PER_JVM`-case, we have this single app composer thing, which runs over the full JVM lifetime and is required to inject the `@Application` into (new) test instances via `doInject(...)`.
Can you add some more details, @rmannibucau ?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tomee] rzo1 merged pull request #767: TOMEE-2977 - Provide a ApplicationComposer JUnit 5 Extension
Posted by GitBox <gi...@apache.org>.
rzo1 merged pull request #767:
URL: https://github.com/apache/tomee/pull/767
--
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:
users@infra.apache.org
[GitHub] [tomee] rmannibucau commented on a change in pull request #767: TOMEE-2977 - Provide a ApplicationComposer JUnit 5 Extension
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on a change in pull request #767:
URL: https://github.com/apache/tomee/pull/767#discussion_r596223276
##########
File path: container/openejb-core/src/main/java/org/apache/openejb/junit/jupiter/ApplicationComposerExtension.java
##########
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit.jupiter;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.apache.openejb.testing.ApplicationComposers;
+import org.apache.openejb.testing.SingleApplicationComposerBase;
+import org.junit.jupiter.api.extension.*;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+
+public class ApplicationComposerExtension extends ApplicationComposerExtensionBase implements BeforeAllCallback, AfterAllCallback, BeforeEachCallback, AfterEachCallback {
+
+ private static final ExtensionContext.Namespace NAMESPACE = ExtensionContext.Namespace.create(ApplicationComposerExtension.class.getName());
+ private static final SingleApplicationComposerBase BASE = new SingleApplicationComposerBase();
+
+ @Override
+ public void beforeAll(ExtensionContext context) throws Exception {
+
+ if (isPerJvm(context)) {
+ BASE.start(context.getTestClass().orElse(null));
Review comment:
guess you can skip @Deprecated ones, they shouldnt be used in junit5 module anyway (new code with deprecated API sounds fishy from the IDE ;)).
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tomee] rmannibucau commented on a change in pull request #767: TOMEE-2977 - Provide a ApplicationComposer JUnit 5 Extension
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on a change in pull request #767:
URL: https://github.com/apache/tomee/pull/767#discussion_r595343572
##########
File path: container/openejb-core/src/main/java/org/apache/openejb/junit/jupiter/ApplicationComposerExtension.java
##########
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit.jupiter;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.apache.openejb.testing.ApplicationComposers;
+import org.apache.openejb.testing.SingleApplicationComposerBase;
+import org.junit.jupiter.api.extension.*;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+
+public class ApplicationComposerExtension extends ApplicationComposerExtensionBase implements BeforeAllCallback, AfterAllCallback, BeforeEachCallback, AfterEachCallback {
+
+ private static final ExtensionContext.Namespace NAMESPACE = ExtensionContext.Namespace.create(ApplicationComposerExtension.class.getName());
+ private static final SingleApplicationComposerBase BASE = new SingleApplicationComposerBase();
+
+ @Override
+ public void beforeAll(ExtensionContext context) throws Exception {
+
+ if (isPerJvm(context)) {
Review comment:
If you have per jvm and per test/class at the same time what happens? Options I see:
1. test if per jvm is started and fail perall/pereach cases
2. make it work (we did in meecrowave, it is mainly a matter of having a JVM singleton/lock to switch the tccl for the bootstrap time + a few singleton fixes)
I'd start with 1 keeping 2 in mind (since it requires more work) so something like 'if !JVM && BASE.isStarted() => fail', isStarted is just matter of testing one of the atomic ref of the single instance.
##########
File path: container/openejb-core/src/main/java/org/apache/openejb/junit/jupiter/ApplicationComposerExtension.java
##########
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit.jupiter;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.apache.openejb.testing.ApplicationComposers;
+import org.apache.openejb.testing.SingleApplicationComposerBase;
+import org.junit.jupiter.api.extension.*;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+
+public class ApplicationComposerExtension extends ApplicationComposerExtensionBase implements BeforeAllCallback, AfterAllCallback, BeforeEachCallback, AfterEachCallback {
+
+ private static final ExtensionContext.Namespace NAMESPACE = ExtensionContext.Namespace.create(ApplicationComposerExtension.class.getName());
+ private static final SingleApplicationComposerBase BASE = new SingleApplicationComposerBase();
+
+ @Override
+ public void beforeAll(ExtensionContext context) throws Exception {
+
+ if (isPerJvm(context)) {
+ BASE.start(context.getTestClass().orElse(null));
+ } else if (isPerAll(context)) {
+ doStart(context);
+ } else if (isPerDefault(context)) {
+ if (isPerClass(context)) {
+ doStart(context);
+ }
+ }
+
+ if (isPerClass(context)) {
+ doInject(context);
+ }
+ }
+
+ @Override
+ public void afterAll(ExtensionContext context) throws Exception {
+ if (isPerJvm(context) || isPerAll(context)) {
+ doRelease(context);
+ } else if (isPerDefault(context) && isPerClass(context)) {
+ doRelease(context);
+ }
+ }
+
+
+ @Override
+ public void beforeEach(ExtensionContext context) {
+
+ if (isPerEach(context)) {
+ doStart(context);
+ } else if (isPerDefault(context) && !isPerClass(context)) {
+ doStart(context);
+ }
+
+ if (!isPerClass(context)) {
+ doInject(context);
+ }
+ }
+
+ @Override
+ public void afterEach(ExtensionContext context) throws Exception {
+ if (isPerEach(context)) {
+ doRelease(context);
+ } else if (isPerDefault(context) && !isPerClass(context)) {
+ doRelease(context);
+ }
+ }
+
+ private void doRelease(final ExtensionContext extensionContext) throws Exception {
+ if (isPerJvm(extensionContext)) {
+ //FIXME how to release / close after last test class execution?
Review comment:
should be done with the shutdown hook
this implementation does NOT work so it likely means it misses a test which should fail (like chaining 2 test classes and ensuring it uses the same container)
##########
File path: container/openejb-core/src/main/java/org/apache/openejb/junit/jupiter/ApplicationComposerExtension.java
##########
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit.jupiter;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.apache.openejb.testing.ApplicationComposers;
+import org.apache.openejb.testing.SingleApplicationComposerBase;
+import org.junit.jupiter.api.extension.*;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+
+public class ApplicationComposerExtension extends ApplicationComposerExtensionBase implements BeforeAllCallback, AfterAllCallback, BeforeEachCallback, AfterEachCallback {
+
+ private static final ExtensionContext.Namespace NAMESPACE = ExtensionContext.Namespace.create(ApplicationComposerExtension.class.getName());
+ private static final SingleApplicationComposerBase BASE = new SingleApplicationComposerBase();
+
+ @Override
+ public void beforeAll(ExtensionContext context) throws Exception {
+
+ if (isPerJvm(context)) {
+ BASE.start(context.getTestClass().orElse(null));
+ } else if (isPerAll(context)) {
+ doStart(context);
+ } else if (isPerDefault(context)) {
+ if (isPerClass(context)) {
+ doStart(context);
+ }
+ }
+
+ if (isPerClass(context)) {
+ doInject(context);
+ }
+ }
+
+ @Override
+ public void afterAll(ExtensionContext context) throws Exception {
+ if (isPerJvm(context) || isPerAll(context)) {
+ doRelease(context);
+ } else if (isPerDefault(context) && isPerClass(context)) {
+ doRelease(context);
+ }
+ }
+
+
+ @Override
+ public void beforeEach(ExtensionContext context) {
+
+ if (isPerEach(context)) {
Review comment:
Think I'd try to merge tests which would also enable to move the doInject next the doStart which would help to simplify the flow.
Open question: if beforeAll adds something specific in the store then if it is not in the store there it means it is not "all" or "per jvm" so you start and inject there no? Isn't it easier to just test an "all" marker for each impl?
##########
File path: container/openejb-core/src/main/java/org/apache/openejb/junit/jupiter/ApplicationComposerExtension.java
##########
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit.jupiter;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.apache.openejb.testing.ApplicationComposers;
+import org.apache.openejb.testing.SingleApplicationComposerBase;
+import org.junit.jupiter.api.extension.*;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+
+public class ApplicationComposerExtension extends ApplicationComposerExtensionBase implements BeforeAllCallback, AfterAllCallback, BeforeEachCallback, AfterEachCallback {
+
+ private static final ExtensionContext.Namespace NAMESPACE = ExtensionContext.Namespace.create(ApplicationComposerExtension.class.getName());
+ private static final SingleApplicationComposerBase BASE = new SingleApplicationComposerBase();
+
+ @Override
+ public void beforeAll(ExtensionContext context) throws Exception {
+
+ if (isPerJvm(context)) {
+ BASE.start(context.getTestClass().orElse(null));
+ } else if (isPerAll(context)) {
+ doStart(context);
+ } else if (isPerDefault(context)) {
+ if (isPerClass(context)) {
+ doStart(context);
+ }
+ }
+
+ if (isPerClass(context)) {
+ doInject(context);
+ }
+ }
+
+ @Override
+ public void afterAll(ExtensionContext context) throws Exception {
+ if (isPerJvm(context) || isPerAll(context)) {
+ doRelease(context);
+ } else if (isPerDefault(context) && isPerClass(context)) {
+ doRelease(context);
+ }
+ }
+
+
+ @Override
+ public void beforeEach(ExtensionContext context) {
+
+ if (isPerEach(context)) {
+ doStart(context);
+ } else if (isPerDefault(context) && !isPerClass(context)) {
+ doStart(context);
+ }
+
+ if (!isPerClass(context)) {
+ doInject(context);
+ }
+ }
+
+ @Override
+ public void afterEach(ExtensionContext context) throws Exception {
+ if (isPerEach(context)) {
Review comment:
I'd use the same AfterEachReleaser hack (side note: name is not great but you get the idea ;)).
##########
File path: container/openejb-core/src/main/java/org/apache/openejb/junit/jupiter/ApplicationComposerExtension.java
##########
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit.jupiter;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.apache.openejb.testing.ApplicationComposers;
+import org.apache.openejb.testing.SingleApplicationComposerBase;
+import org.junit.jupiter.api.extension.*;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+
+public class ApplicationComposerExtension extends ApplicationComposerExtensionBase implements BeforeAllCallback, AfterAllCallback, BeforeEachCallback, AfterEachCallback {
+
+ private static final ExtensionContext.Namespace NAMESPACE = ExtensionContext.Namespace.create(ApplicationComposerExtension.class.getName());
+ private static final SingleApplicationComposerBase BASE = new SingleApplicationComposerBase();
+
+ @Override
+ public void beforeAll(ExtensionContext context) throws Exception {
+
+ if (isPerJvm(context)) {
+ BASE.start(context.getTestClass().orElse(null));
Review comment:
do you want to check there is no @Module/@Classes/... or so in the class? it would be a misuse of the API
##########
File path: container/openejb-core/src/main/java/org/apache/openejb/junit/jupiter/ApplicationComposerExtension.java
##########
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit.jupiter;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.apache.openejb.testing.ApplicationComposers;
+import org.apache.openejb.testing.SingleApplicationComposerBase;
+import org.junit.jupiter.api.extension.*;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+
+public class ApplicationComposerExtension extends ApplicationComposerExtensionBase implements BeforeAllCallback, AfterAllCallback, BeforeEachCallback, AfterEachCallback {
+
+ private static final ExtensionContext.Namespace NAMESPACE = ExtensionContext.Namespace.create(ApplicationComposerExtension.class.getName());
+ private static final SingleApplicationComposerBase BASE = new SingleApplicationComposerBase();
+
+ @Override
+ public void beforeAll(ExtensionContext context) throws Exception {
+
+ if (isPerJvm(context)) {
+ BASE.start(context.getTestClass().orElse(null));
+ } else if (isPerAll(context)) {
+ doStart(context);
+ } else if (isPerDefault(context)) {
+ if (isPerClass(context)) {
+ doStart(context);
+ }
+ }
+
+ if (isPerClass(context)) {
+ doInject(context);
+ }
+ }
+
+ @Override
+ public void afterAll(ExtensionContext context) throws Exception {
+ if (isPerJvm(context) || isPerAll(context)) {
Review comment:
perjvm => do not release?
##########
File path: container/openejb-core/pom.xml
##########
@@ -580,6 +580,22 @@
<groupId>org.apache.xbean</groupId>
<artifactId>xbean-bundleutils</artifactId>
</dependency>
+ <dependency>
Review comment:
open point: should it go in a new openejb-junit5 module? Having testing stack in openejb-core is an inheritance but bothering at some point so maybe time to clean it up? wdyt?
##########
File path: tomee/tomee-embedded/src/main/java/org/apache/tomee/embedded/junit/jupiter/TomEEEmbeddedExtension.java
##########
@@ -0,0 +1,86 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tomee.embedded.junit.jupiter;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.apache.openejb.junit.jupiter.ApplicationComposerExtensionBase;
+import org.apache.tomee.embedded.junit.TomEEEmbeddedBase;
+import org.junit.jupiter.api.TestInstance;
+import org.junit.jupiter.api.extension.*;
Review comment:
wildcard? ;)
##########
File path: pom.xml
##########
@@ -97,8 +97,7 @@
<tomee.version>${project.version}</tomee.version>
<maven.compiler.source>1.8</maven.compiler.source>
<maven.compiler.target>1.8</maven.compiler.target>
- <surefire.version>2.21.0</surefire.version>
- <surefire.junit5.version>3.0.0-M5</surefire.junit5.version>
+ <surefire.version>2.22.1</surefire.version>
Review comment:
don't downgrade, as mentionned iit can fake it works with junit5 whereas it actually does not
##########
File path: container/openejb-core/src/main/java/org/apache/openejb/junit/jupiter/ApplicationComposerExtension.java
##########
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit.jupiter;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.apache.openejb.testing.ApplicationComposers;
+import org.apache.openejb.testing.SingleApplicationComposerBase;
+import org.junit.jupiter.api.extension.*;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+
+public class ApplicationComposerExtension extends ApplicationComposerExtensionBase implements BeforeAllCallback, AfterAllCallback, BeforeEachCallback, AfterEachCallback {
+
+ private static final ExtensionContext.Namespace NAMESPACE = ExtensionContext.Namespace.create(ApplicationComposerExtension.class.getName());
+ private static final SingleApplicationComposerBase BASE = new SingleApplicationComposerBase();
+
+ @Override
+ public void beforeAll(ExtensionContext context) throws Exception {
+
+ if (isPerJvm(context)) {
+ BASE.start(context.getTestClass().orElse(null));
+ } else if (isPerAll(context)) {
+ doStart(context);
+ } else if (isPerDefault(context)) {
+ if (isPerClass(context)) {
+ doStart(context);
+ }
+ }
+
+ if (isPerClass(context)) {
Review comment:
not sure it is intended but previous code makes it possible to not have started anything there and if so it must not inject anything
I suspect not starting at all is a bug so maybe rework the if/else flow to ensure we start?
##########
File path: examples/junit5-application-composer/pom.xml
##########
@@ -0,0 +1,120 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+<!-- $Rev: 636494 $ $Date: 2008-03-12 21:24:02 +0100 (Wed, 12 Mar 2008) $ -->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+ <groupId>org.superbiz</groupId>
+ <artifactId>junit5-application-composer</artifactId>
+ <packaging>jar</packaging>
+ <version>8.0.7-SNAPSHOT</version>
+ <name>TomEE :: Examples :: JUnit 5 :: Application Composer</name>
+ <properties>
+ <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+ </properties>
+ <build>
+ <defaultGoal>install</defaultGoal>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-compiler-plugin</artifactId>
+ <version>3.5.1</version>
+ <configuration>
+ <source>1.8</source>
+ <target>1.8</target>
+ </configuration>
+ </plugin>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-surefire-plugin</artifactId>
+ <version>2.22.1</version>
Review comment:
use 3.0.0-M5 please, previous versions have all pitfalls with junit5
##########
File path: container/openejb-core/src/main/java/org/apache/openejb/junit/jupiter/ApplicationComposerExtension.java
##########
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit.jupiter;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.apache.openejb.testing.ApplicationComposers;
+import org.apache.openejb.testing.SingleApplicationComposerBase;
+import org.junit.jupiter.api.extension.*;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+
+public class ApplicationComposerExtension extends ApplicationComposerExtensionBase implements BeforeAllCallback, AfterAllCallback, BeforeEachCallback, AfterEachCallback {
+
+ private static final ExtensionContext.Namespace NAMESPACE = ExtensionContext.Namespace.create(ApplicationComposerExtension.class.getName());
+ private static final SingleApplicationComposerBase BASE = new SingleApplicationComposerBase();
+
+ @Override
+ public void beforeAll(ExtensionContext context) throws Exception {
+
+ if (isPerJvm(context)) {
+ BASE.start(context.getTestClass().orElse(null));
+ } else if (isPerAll(context)) {
+ doStart(context);
+ } else if (isPerDefault(context)) {
+ if (isPerClass(context)) {
+ doStart(context);
+ }
+ }
+
+ if (isPerClass(context)) {
+ doInject(context);
+ }
+ }
+
+ @Override
+ public void afterAll(ExtensionContext context) throws Exception {
+ if (isPerJvm(context) || isPerAll(context)) {
+ doRelease(context);
+ } else if (isPerDefault(context) && isPerClass(context)) {
+ doRelease(context);
+ }
+ }
+
+
+ @Override
+ public void beforeEach(ExtensionContext context) {
+
+ if (isPerEach(context)) {
+ doStart(context);
+ } else if (isPerDefault(context) && !isPerClass(context)) {
+ doStart(context);
+ }
+
+ if (!isPerClass(context)) {
+ doInject(context);
+ }
+ }
+
+ @Override
+ public void afterEach(ExtensionContext context) throws Exception {
+ if (isPerEach(context)) {
+ doRelease(context);
+ } else if (isPerDefault(context) && !isPerClass(context)) {
+ doRelease(context);
+ }
+ }
+
+ private void doRelease(final ExtensionContext extensionContext) throws Exception {
+ if (isPerJvm(extensionContext)) {
+ //FIXME how to release / close after last test class execution?
+ BASE.close();
+ } else {
+ extensionContext.getStore(NAMESPACE).get(ApplicationComposers.class, ApplicationComposers.class).after();
+ }
+ }
+
+ private void doStart(final ExtensionContext extensionContext) {
+
+ Optional<Class<?>> oClazz = extensionContext.getTestClass();
+
+ if (!oClazz.isPresent()) {
+ throw new OpenEJBRuntimeException("Could not get test class from the given extension context.");
+ }
+
+ extensionContext.getStore(NAMESPACE).put(ApplicationComposers.class,
+ new ApplicationComposers(oClazz.get(), getAdditionalModules(oClazz.get())));
+
+ }
+
+ private void doInject(final ExtensionContext extensionContext) {
+ Optional<TestInstances> oTestInstances = extensionContext.getTestInstances();
Review comment:
orElseThrow ;)
##########
File path: container/openejb-core/src/main/java/org/apache/openejb/junit/jupiter/ApplicationComposerExtension.java
##########
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit.jupiter;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.apache.openejb.testing.ApplicationComposers;
+import org.apache.openejb.testing.SingleApplicationComposerBase;
+import org.junit.jupiter.api.extension.*;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+
+public class ApplicationComposerExtension extends ApplicationComposerExtensionBase implements BeforeAllCallback, AfterAllCallback, BeforeEachCallback, AfterEachCallback {
+
+ private static final ExtensionContext.Namespace NAMESPACE = ExtensionContext.Namespace.create(ApplicationComposerExtension.class.getName());
+ private static final SingleApplicationComposerBase BASE = new SingleApplicationComposerBase();
+
+ @Override
+ public void beforeAll(ExtensionContext context) throws Exception {
+
+ if (isPerJvm(context)) {
+ BASE.start(context.getTestClass().orElse(null));
+ } else if (isPerAll(context)) {
+ doStart(context);
+ } else if (isPerDefault(context)) {
+ if (isPerClass(context)) {
+ doStart(context);
+ }
+ }
+
+ if (isPerClass(context)) {
+ doInject(context);
+ }
+ }
+
+ @Override
+ public void afterAll(ExtensionContext context) throws Exception {
+ if (isPerJvm(context) || isPerAll(context)) {
+ doRelease(context);
+ } else if (isPerDefault(context) && isPerClass(context)) {
+ doRelease(context);
+ }
+ }
+
+
+ @Override
+ public void beforeEach(ExtensionContext context) {
+
+ if (isPerEach(context)) {
+ doStart(context);
+ } else if (isPerDefault(context) && !isPerClass(context)) {
+ doStart(context);
+ }
+
+ if (!isPerClass(context)) {
+ doInject(context);
+ }
+ }
+
+ @Override
+ public void afterEach(ExtensionContext context) throws Exception {
+ if (isPerEach(context)) {
+ doRelease(context);
+ } else if (isPerDefault(context) && !isPerClass(context)) {
+ doRelease(context);
+ }
+ }
+
+ private void doRelease(final ExtensionContext extensionContext) throws Exception {
+ if (isPerJvm(extensionContext)) {
+ //FIXME how to release / close after last test class execution?
+ BASE.close();
+ } else {
+ extensionContext.getStore(NAMESPACE).get(ApplicationComposers.class, ApplicationComposers.class).after();
+ }
+ }
+
+ private void doStart(final ExtensionContext extensionContext) {
+
+ Optional<Class<?>> oClazz = extensionContext.getTestClass();
Review comment:
> final Class<?> oClazz = extensionContext.getTestClass().orElseThrow(() ->new OpenEJBRuntimeException("Could not get test class from the given extension context."));
##########
File path: examples/junit5-application-composer/pom.xml
##########
@@ -0,0 +1,120 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+<!-- $Rev: 636494 $ $Date: 2008-03-12 21:24:02 +0100 (Wed, 12 Mar 2008) $ -->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+ <groupId>org.superbiz</groupId>
+ <artifactId>junit5-application-composer</artifactId>
+ <packaging>jar</packaging>
+ <version>8.0.7-SNAPSHOT</version>
+ <name>TomEE :: Examples :: JUnit 5 :: Application Composer</name>
+ <properties>
+ <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+ </properties>
+ <build>
+ <defaultGoal>install</defaultGoal>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-compiler-plugin</artifactId>
+ <version>3.5.1</version>
+ <configuration>
+ <source>1.8</source>
+ <target>1.8</target>
+ </configuration>
+ </plugin>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-surefire-plugin</artifactId>
+ <version>2.22.1</version>
+ <configuration>
+ <argLine>-Djdk.attach.allowAttachSelf</argLine>
+ </configuration>
+ </plugin>
+ <plugin>
+ <groupId>org.tomitribe.transformer</groupId>
+ <artifactId>org.eclipse.transformer.maven</artifactId>
+ <version>0.1.1a</version>
+ <configuration>
+ <classifier>jakartaee9</classifier>
+ </configuration>
+ <executions>
+ <execution>
+ <goals>
+ <goal>run</goal>
+ </goals>
+ <phase>package</phase>
+ </execution>
+ </executions>
+ </plugin>
+ </plugins>
+ </build>
+ <repositories>
+ <repository>
+ <id>apache-m2-snapshot</id>
+ <name>Apache Snapshot Repository</name>
+ <url>https://repository.apache.org/content/groups/snapshots</url>
+ </repository>
+ </repositories>
+ <dependencies>
+ <dependency>
+ <groupId>org.apache.tomee</groupId>
+ <artifactId>javaee-api</artifactId>
+ <version>[8.0,)</version>
+ <scope>provided</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.junit.jupiter</groupId>
+ <artifactId>junit-jupiter-api</artifactId>
Review comment:
<artifactId>junit-jupiter</artifactId> as single junit5 dep
##########
File path: container/openejb-core/src/main/java/org/apache/openejb/junit/jupiter/ApplicationComposerExtension.java
##########
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit.jupiter;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.apache.openejb.testing.ApplicationComposers;
+import org.apache.openejb.testing.SingleApplicationComposerBase;
+import org.junit.jupiter.api.extension.*;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+
+public class ApplicationComposerExtension extends ApplicationComposerExtensionBase implements BeforeAllCallback, AfterAllCallback, BeforeEachCallback, AfterEachCallback {
+
+ private static final ExtensionContext.Namespace NAMESPACE = ExtensionContext.Namespace.create(ApplicationComposerExtension.class.getName());
+ private static final SingleApplicationComposerBase BASE = new SingleApplicationComposerBase();
+
+ @Override
+ public void beforeAll(ExtensionContext context) throws Exception {
+
+ if (isPerJvm(context)) {
+ BASE.start(context.getTestClass().orElse(null));
+ } else if (isPerAll(context)) {
+ doStart(context);
+ } else if (isPerDefault(context)) {
+ if (isPerClass(context)) {
+ doStart(context);
+ }
+ }
+
+ if (isPerClass(context)) {
+ doInject(context);
+ }
+ }
+
+ @Override
+ public void afterAll(ExtensionContext context) throws Exception {
+ if (isPerJvm(context) || isPerAll(context)) {
+ doRelease(context);
+ } else if (isPerDefault(context) && isPerClass(context)) {
Review comment:
my 2 cts would be to create an AfterAllReleaser {void run()} and put it in the context in beforeAll next to the related start logic.
Then afterXXX is just context.getNamespace(...).getStore().get(AfterAllReleaser.class, AfterAllReleaser.class).ifPresent(AfterAllReleaser::run);
Same can apply to afterEach.
Big advantage is you don't duplicate all tests, it is symmetric by construction.
##########
File path: container/openejb-core/src/main/java/org/apache/openejb/junit/jupiter/ApplicationComposerExtension.java
##########
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit.jupiter;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.apache.openejb.testing.ApplicationComposers;
+import org.apache.openejb.testing.SingleApplicationComposerBase;
+import org.junit.jupiter.api.extension.*;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+
+public class ApplicationComposerExtension extends ApplicationComposerExtensionBase implements BeforeAllCallback, AfterAllCallback, BeforeEachCallback, AfterEachCallback {
+
+ private static final ExtensionContext.Namespace NAMESPACE = ExtensionContext.Namespace.create(ApplicationComposerExtension.class.getName());
+ private static final SingleApplicationComposerBase BASE = new SingleApplicationComposerBase();
+
+ @Override
+ public void beforeAll(ExtensionContext context) throws Exception {
+
+ if (isPerJvm(context)) {
+ BASE.start(context.getTestClass().orElse(null));
+ } else if (isPerAll(context)) {
+ doStart(context);
+ } else if (isPerDefault(context)) {
+ if (isPerClass(context)) {
+ doStart(context);
+ }
+ }
+
+ if (isPerClass(context)) {
+ doInject(context);
+ }
+ }
+
+ @Override
+ public void afterAll(ExtensionContext context) throws Exception {
+ if (isPerJvm(context) || isPerAll(context)) {
+ doRelease(context);
+ } else if (isPerDefault(context) && isPerClass(context)) {
+ doRelease(context);
+ }
+ }
+
+
+ @Override
+ public void beforeEach(ExtensionContext context) {
+
+ if (isPerEach(context)) {
+ doStart(context);
+ } else if (isPerDefault(context) && !isPerClass(context)) {
+ doStart(context);
+ }
+
+ if (!isPerClass(context)) {
+ doInject(context);
+ }
+ }
+
+ @Override
+ public void afterEach(ExtensionContext context) throws Exception {
+ if (isPerEach(context)) {
+ doRelease(context);
+ } else if (isPerDefault(context) && !isPerClass(context)) {
+ doRelease(context);
+ }
+ }
+
+ private void doRelease(final ExtensionContext extensionContext) throws Exception {
+ if (isPerJvm(extensionContext)) {
+ //FIXME how to release / close after last test class execution?
+ BASE.close();
+ } else {
+ extensionContext.getStore(NAMESPACE).get(ApplicationComposers.class, ApplicationComposers.class).after();
+ }
+ }
+
+ private void doStart(final ExtensionContext extensionContext) {
+
+ Optional<Class<?>> oClazz = extensionContext.getTestClass();
+
+ if (!oClazz.isPresent()) {
+ throw new OpenEJBRuntimeException("Could not get test class from the given extension context.");
+ }
+
+ extensionContext.getStore(NAMESPACE).put(ApplicationComposers.class,
+ new ApplicationComposers(oClazz.get(), getAdditionalModules(oClazz.get())));
+
+ }
+
+ private void doInject(final ExtensionContext extensionContext) {
+ Optional<TestInstances> oTestInstances = extensionContext.getTestInstances();
+
+ if (!oTestInstances.isPresent()) {
+ throw new OpenEJBRuntimeException("No test instances available for the given extension context.");
+ }
+
+ List<Object> testInstances = oTestInstances.get().getAllInstances();
+
+ if (isPerJvm(extensionContext)) {
+ testInstances.forEach(t -> {
+ try {
+ BASE.composerInject(t);
+ } catch (Exception e) {
+ throw new OpenEJBRuntimeException(e);
+ }
+ });
+ } else {
+ ApplicationComposers delegate = extensionContext.getStore(NAMESPACE)
+ .get(ApplicationComposers.class, ApplicationComposers.class);
+
+ testInstances.forEach(t -> {
+ try {
+ delegate.before(t);
Review comment:
this does not injects but starts a container so this is not supposed to work - or at least as expected, maybe add a test to validate this case
##########
File path: container/openejb-core/src/main/java/org/apache/openejb/junit/jupiter/ApplicationComposerExtension.java
##########
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit.jupiter;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.apache.openejb.testing.ApplicationComposers;
+import org.apache.openejb.testing.SingleApplicationComposerBase;
+import org.junit.jupiter.api.extension.*;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+
+public class ApplicationComposerExtension extends ApplicationComposerExtensionBase implements BeforeAllCallback, AfterAllCallback, BeforeEachCallback, AfterEachCallback {
+
+ private static final ExtensionContext.Namespace NAMESPACE = ExtensionContext.Namespace.create(ApplicationComposerExtension.class.getName());
+ private static final SingleApplicationComposerBase BASE = new SingleApplicationComposerBase();
+
+ @Override
+ public void beforeAll(ExtensionContext context) throws Exception {
+
+ if (isPerJvm(context)) {
+ BASE.start(context.getTestClass().orElse(null));
+ } else if (isPerAll(context)) {
+ doStart(context);
+ } else if (isPerDefault(context)) {
+ if (isPerClass(context)) {
+ doStart(context);
+ }
+ }
+
+ if (isPerClass(context)) {
+ doInject(context);
+ }
+ }
+
+ @Override
+ public void afterAll(ExtensionContext context) throws Exception {
+ if (isPerJvm(context) || isPerAll(context)) {
+ doRelease(context);
+ } else if (isPerDefault(context) && isPerClass(context)) {
+ doRelease(context);
+ }
+ }
+
+
+ @Override
+ public void beforeEach(ExtensionContext context) {
+
+ if (isPerEach(context)) {
+ doStart(context);
+ } else if (isPerDefault(context) && !isPerClass(context)) {
+ doStart(context);
+ }
+
+ if (!isPerClass(context)) {
+ doInject(context);
+ }
+ }
+
+ @Override
+ public void afterEach(ExtensionContext context) throws Exception {
+ if (isPerEach(context)) {
+ doRelease(context);
+ } else if (isPerDefault(context) && !isPerClass(context)) {
+ doRelease(context);
+ }
+ }
+
+ private void doRelease(final ExtensionContext extensionContext) throws Exception {
+ if (isPerJvm(extensionContext)) {
+ //FIXME how to release / close after last test class execution?
+ BASE.close();
+ } else {
+ extensionContext.getStore(NAMESPACE).get(ApplicationComposers.class, ApplicationComposers.class).after();
+ }
+ }
+
+ private void doStart(final ExtensionContext extensionContext) {
+
+ Optional<Class<?>> oClazz = extensionContext.getTestClass();
+
+ if (!oClazz.isPresent()) {
+ throw new OpenEJBRuntimeException("Could not get test class from the given extension context.");
+ }
+
+ extensionContext.getStore(NAMESPACE).put(ApplicationComposers.class,
+ new ApplicationComposers(oClazz.get(), getAdditionalModules(oClazz.get())));
+
+ }
+
+ private void doInject(final ExtensionContext extensionContext) {
+ Optional<TestInstances> oTestInstances = extensionContext.getTestInstances();
+
+ if (!oTestInstances.isPresent()) {
+ throw new OpenEJBRuntimeException("No test instances available for the given extension context.");
+ }
+
+ List<Object> testInstances = oTestInstances.get().getAllInstances();
+
+ if (isPerJvm(extensionContext)) {
+ testInstances.forEach(t -> {
+ try {
+ BASE.composerInject(t);
+ } catch (Exception e) {
+ throw new OpenEJBRuntimeException(e);
+ }
+ });
+ } else {
+ ApplicationComposers delegate = extensionContext.getStore(NAMESPACE)
+ .get(ApplicationComposers.class, ApplicationComposers.class);
+
+ testInstances.forEach(t -> {
+ try {
+ delegate.before(t);
+ } catch (Exception e) {
+ throw new OpenEJBRuntimeException(e);
+ }
+ });
+ }
+ }
+
+ private Object[] getAdditionalModules(final Class<?> clazz) {
+ return Arrays.stream(clazz.getClasses())
Review comment:
this likely do not do what is expected and can be random (thinking to @Nested tests of junit5).
ApplicationComposerRule shows the idea: you pass the list of additional modules.
In junit5 it can be:
> @RegisterExtension ApplicationComposerExtension ext = new ApplicationComposerExtension(new MyApp());
it enables to define the app outside the test and/or to compose subapps definitions (like a MyStack() + finish the spec in the test)
##########
File path: tomee/tomee-embedded/src/main/java/org/apache/tomee/embedded/junit/jupiter/TomEEEmbeddedExtension.java
##########
@@ -0,0 +1,86 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tomee.embedded.junit.jupiter;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.apache.openejb.junit.jupiter.ApplicationComposerExtensionBase;
+import org.apache.tomee.embedded.junit.TomEEEmbeddedBase;
+import org.junit.jupiter.api.TestInstance;
+import org.junit.jupiter.api.extension.*;
+
+import java.util.List;
+import java.util.Optional;
+
+public class TomEEEmbeddedExtension extends ApplicationComposerExtensionBase implements BeforeAllCallback, AfterAllCallback, BeforeEachCallback, AfterEachCallback {
Review comment:
maybe also define the @RunWithTomEEEmbedded
Also wonder if ApplicationComposerExtensionBase is needed, as seen in app composer case, you have beforeAll to test if you are in JVM or per class mode, then all other tests are deduced from the content of the context store so not sure it is good to define a dependency between both modules just to test a scope. (from an artifact/dep point of view we should be able to run tomee embedded without application composer impl, only its org.apache.openejb.testing API)
##########
File path: container/openejb-core/src/main/java/org/apache/openejb/junit/jupiter/ApplicationComposerExtension.java
##########
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit.jupiter;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.apache.openejb.testing.ApplicationComposers;
+import org.apache.openejb.testing.SingleApplicationComposerBase;
+import org.junit.jupiter.api.extension.*;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+
+public class ApplicationComposerExtension extends ApplicationComposerExtensionBase implements BeforeAllCallback, AfterAllCallback, BeforeEachCallback, AfterEachCallback {
+
+ private static final ExtensionContext.Namespace NAMESPACE = ExtensionContext.Namespace.create(ApplicationComposerExtension.class.getName());
+ private static final SingleApplicationComposerBase BASE = new SingleApplicationComposerBase();
+
+ @Override
+ public void beforeAll(ExtensionContext context) throws Exception {
+
+ if (isPerJvm(context)) {
+ BASE.start(context.getTestClass().orElse(null));
+ } else if (isPerAll(context)) {
+ doStart(context);
+ } else if (isPerDefault(context)) {
+ if (isPerClass(context)) {
+ doStart(context);
+ }
+ }
+
+ if (isPerClass(context)) {
+ doInject(context);
+ }
+ }
+
+ @Override
+ public void afterAll(ExtensionContext context) throws Exception {
+ if (isPerJvm(context) || isPerAll(context)) {
+ doRelease(context);
+ } else if (isPerDefault(context) && isPerClass(context)) {
+ doRelease(context);
+ }
+ }
+
+
+ @Override
+ public void beforeEach(ExtensionContext context) {
+
+ if (isPerEach(context)) {
+ doStart(context);
+ } else if (isPerDefault(context) && !isPerClass(context)) {
+ doStart(context);
+ }
+
+ if (!isPerClass(context)) {
+ doInject(context);
+ }
+ }
+
+ @Override
+ public void afterEach(ExtensionContext context) throws Exception {
+ if (isPerEach(context)) {
+ doRelease(context);
+ } else if (isPerDefault(context) && !isPerClass(context)) {
+ doRelease(context);
+ }
+ }
+
+ private void doRelease(final ExtensionContext extensionContext) throws Exception {
+ if (isPerJvm(extensionContext)) {
+ //FIXME how to release / close after last test class execution?
+ BASE.close();
+ } else {
+ extensionContext.getStore(NAMESPACE).get(ApplicationComposers.class, ApplicationComposers.class).after();
+ }
+ }
+
+ private void doStart(final ExtensionContext extensionContext) {
+
+ Optional<Class<?>> oClazz = extensionContext.getTestClass();
+
+ if (!oClazz.isPresent()) {
+ throw new OpenEJBRuntimeException("Could not get test class from the given extension context.");
+ }
+
+ extensionContext.getStore(NAMESPACE).put(ApplicationComposers.class,
+ new ApplicationComposers(oClazz.get(), getAdditionalModules(oClazz.get())));
+
+ }
+
+ private void doInject(final ExtensionContext extensionContext) {
+ Optional<TestInstances> oTestInstances = extensionContext.getTestInstances();
+
+ if (!oTestInstances.isPresent()) {
+ throw new OpenEJBRuntimeException("No test instances available for the given extension context.");
+ }
+
+ List<Object> testInstances = oTestInstances.get().getAllInstances();
+
+ if (isPerJvm(extensionContext)) {
+ testInstances.forEach(t -> {
+ try {
+ BASE.composerInject(t);
+ } catch (Exception e) {
+ throw new OpenEJBRuntimeException(e);
+ }
+ });
+ } else {
+ ApplicationComposers delegate = extensionContext.getStore(NAMESPACE)
+ .get(ApplicationComposers.class, ApplicationComposers.class);
+
+ testInstances.forEach(t -> {
+ try {
+ delegate.before(t);
+ } catch (Exception e) {
+ throw new OpenEJBRuntimeException(e);
+ }
+ });
+ }
+ }
+
+ private Object[] getAdditionalModules(final Class<?> clazz) {
+ return Arrays.stream(clazz.getClasses())
+ .map(this::newInstance)
+ .filter(Objects::nonNull)
+ .toArray();
+ }
+
+ private Object newInstance(final Class<?> clazz) {
+ try {
+ return clazz.newInstance();
Review comment:
> clazz.getConstructor().newInstance()
blame java, not me ;)
(only relevant if you decide to still use nested classes which is not needed regarding previous comment)
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tomee] rzo1 commented on a change in pull request #767: TOMEE-2977 - Provide a ApplicationComposer JUnit 5 Extension
Posted by GitBox <gi...@apache.org>.
rzo1 commented on a change in pull request #767:
URL: https://github.com/apache/tomee/pull/767#discussion_r595302374
##########
File path: container/openejb-core/src/main/java/org/apache/openejb/junit/jupiter/ApplicationComposerExtensionBase.java
##########
@@ -26,4 +27,36 @@ boolean isPerClass(final ExtensionContext context) {
.map(it -> it.equals(TestInstance.Lifecycle.PER_CLASS))
.orElse(false);
}
+
+ boolean isPerEach(final ExtensionContext context) {
+ return checkMode(context, ExtensionMode.PER_EACH);
+ }
+
+ boolean isPerAll(final ExtensionContext context) {
+ return checkMode(context, ExtensionMode.PER_ALL);
+ }
+
+ boolean isPerJvm(final ExtensionContext context) {
+ return checkMode(context, ExtensionMode.PER_JVM);
+ }
+
+ boolean isPerDefault(final ExtensionContext context) {
+ return checkMode(context, ExtensionMode.AUTO);
+ }
+
+ boolean checkMode(final ExtensionContext context, ExtensionMode extensionMode ) {
+ return extensionMode == getModeFromAnnotation(context);
+ }
+
+ ExtensionMode getModeFromAnnotation(final ExtensionContext context) {
+ if (context.getTestClass().isPresent()) {
+
+ RunWithApplicationComposer a = context.getTestClass().get().getAnnotation(RunWithApplicationComposer.class);
Review comment:
Thx. Changed it.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tomee] rzo1 edited a comment on pull request #767: TOMEE-2977 - Provide a ApplicationComposer JUnit 5 Extension
Posted by GitBox <gi...@apache.org>.
rzo1 edited a comment on pull request #767:
URL: https://github.com/apache/tomee/pull/767#issuecomment-801920909
Should now contain the proposed fixes / enhancements.
Tests are now executed in an additional phase. Let's wait if we receive some additional feedback.
EDIT-1: Rebased the PR ([original commit history can be found here](https://github.com/rzo1/tomee/commits/2977-original-without-rebase)) - Jenkins PR builder was some sort of stuck. Hopefully it will now pick up the PR again.
EDIT-2: Jenkins build has passed. However, GitHub didn't pick up the new status.
--
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:
users@infra.apache.org
[GitHub] [tomee] rmannibucau commented on a change in pull request #767: TOMEE-2977 - Provide a ApplicationComposer JUnit 5 Extension
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on a change in pull request #767:
URL: https://github.com/apache/tomee/pull/767#discussion_r596224991
##########
File path: container/openejb-core/pom.xml
##########
@@ -580,6 +580,22 @@
<groupId>org.apache.xbean</groupId>
<artifactId>xbean-bundleutils</artifactId>
</dependency>
+ <dependency>
Review comment:
Hmm, the facts to consider IMHO are:
1. any junit5 dep must not bring back any junit 4 dependency (this creates weird bugs in test runners for gradle/maven/ide)
2. junit legacy module is completely transversal to app composer so I think we should keep them orthogonal, can need a better naming but also guarantees a better design/consumption
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tomee] rzo1 commented on a change in pull request #767: TOMEE-2977 - Provide a ApplicationComposer JUnit 5 Extension
Posted by GitBox <gi...@apache.org>.
rzo1 commented on a change in pull request #767:
URL: https://github.com/apache/tomee/pull/767#discussion_r590340268
##########
File path: container/openejb-core/src/main/java/org/apache/openejb/junit/jupiter/ApplicationComposerExtension.java
##########
@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit.jupiter;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.apache.openejb.testing.ApplicationComposers;
+import org.junit.jupiter.api.extension.*;
+
+public class ApplicationComposerExtension implements BeforeEachCallback, AfterEachCallback {
Review comment:
Wow. Super fast :+1:
1. I thought about it: Doesn't `ApplicationComposers` require a concrete test instance to inject values and stuff in `delegate.before(...)`?` The required test instance is - afaik - not available when `beforeAll(...)` is called. However, the creation of the `delegate` might be conducted in a `beforeAll(...) call.
2. This is a good idea. I think, it can be done via `context.getRoot().getStore(...)` the `ExtensionContext`.
3. Honestly, I did not thought about this use-case. We may get additional test instances via `context.getRequiredTestInstances().getAllInstances()`. Wdyt?
4. Should be very similar to `SingleApplicationComposerRunner` - I will take a look at it.
5. Good idea. Will change it to `RunWith...` to be inline with the namings in `openejb-junit5-backward`.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tomee] rzo1 commented on a change in pull request #767: TOMEE-2977 - Provide a ApplicationComposer JUnit 5 Extension
Posted by GitBox <gi...@apache.org>.
rzo1 commented on a change in pull request #767:
URL: https://github.com/apache/tomee/pull/767#discussion_r596301148
##########
File path: container/openejb-core/pom.xml
##########
@@ -580,6 +580,22 @@
<groupId>org.apache.xbean</groupId>
<artifactId>xbean-bundleutils</artifactId>
</dependency>
+ <dependency>
Review comment:
1. +1 -> I agree.
2. Yes. It is a totally different approach, so keeping them separated is fine for me. Users may consume them as needed.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tomee] rmannibucau commented on a change in pull request #767: TOMEE-2977 - Provide a ApplicationComposer JUnit 5 Extension
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on a change in pull request #767:
URL: https://github.com/apache/tomee/pull/767#discussion_r590292456
##########
File path: container/openejb-core/src/main/java/org/apache/openejb/junit/jupiter/ApplicationComposerExtension.java
##########
@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit.jupiter;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.apache.openejb.testing.ApplicationComposers;
+import org.junit.jupiter.api.extension.*;
+
+public class ApplicationComposerExtension implements BeforeEachCallback, AfterEachCallback {
Review comment:
A few open questions:
1. isnt it beforeall/afterall callbacks? (way faster)
2. shouldn't "delegate" be stored in ExtensionContext to ensure it is well scoped and not limited to this particular threading+scope which is one of the multiple junit5 cases
3. What about other test instances you can get from the context? should they contribute to the app model?
4. Should it get a "Single"/mono extension (started once per jvm as tomee embedded), it is generally accurate for apps written this way
5. (very minor) I wouldn't mention extension in the annotation "ApplicatioComposerExtension" name (maybe `@WithApplicationComposer` or alike)
hope it makes sense
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tomee] rzo1 commented on a change in pull request #767: TOMEE-2977 - Provide a ApplicationComposer JUnit 5 Extension
Posted by GitBox <gi...@apache.org>.
rzo1 commented on a change in pull request #767:
URL: https://github.com/apache/tomee/pull/767#discussion_r595911922
##########
File path: container/openejb-core/src/main/java/org/apache/openejb/junit/jupiter/ApplicationComposerExtension.java
##########
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit.jupiter;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.apache.openejb.testing.ApplicationComposers;
+import org.apache.openejb.testing.SingleApplicationComposerBase;
+import org.junit.jupiter.api.extension.*;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+
+public class ApplicationComposerExtension extends ApplicationComposerExtensionBase implements BeforeAllCallback, AfterAllCallback, BeforeEachCallback, AfterEachCallback {
+
+ private static final ExtensionContext.Namespace NAMESPACE = ExtensionContext.Namespace.create(ApplicationComposerExtension.class.getName());
+ private static final SingleApplicationComposerBase BASE = new SingleApplicationComposerBase();
+
+ @Override
+ public void beforeAll(ExtensionContext context) throws Exception {
+
+ if (isPerJvm(context)) {
+ BASE.start(context.getTestClass().orElse(null));
+ } else if (isPerAll(context)) {
+ doStart(context);
+ } else if (isPerDefault(context)) {
+ if (isPerClass(context)) {
+ doStart(context);
+ }
+ }
+
+ if (isPerClass(context)) {
+ doInject(context);
+ }
+ }
+
+ @Override
+ public void afterAll(ExtensionContext context) throws Exception {
+ if (isPerJvm(context) || isPerAll(context)) {
+ doRelease(context);
+ } else if (isPerDefault(context) && isPerClass(context)) {
+ doRelease(context);
+ }
+ }
+
+
+ @Override
+ public void beforeEach(ExtensionContext context) {
+
+ if (isPerEach(context)) {
Review comment:
I think, I do not get the idea of the "all"-marker ? Is it about simplyfing the different conditions (which are some how reworked now but maybe subject to change)
Maybe you can add some more context, @rmannibucau ?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tomee] rzo1 commented on a change in pull request #767: TOMEE-2977 - Provide a ApplicationComposer JUnit 5 Extension
Posted by GitBox <gi...@apache.org>.
rzo1 commented on a change in pull request #767:
URL: https://github.com/apache/tomee/pull/767#discussion_r595487810
##########
File path: tomee/tomee-embedded/src/main/java/org/apache/tomee/embedded/junit/jupiter/TomEEEmbeddedExtension.java
##########
@@ -0,0 +1,86 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tomee.embedded.junit.jupiter;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.apache.openejb.junit.jupiter.ApplicationComposerExtensionBase;
+import org.apache.tomee.embedded.junit.TomEEEmbeddedBase;
+import org.junit.jupiter.api.TestInstance;
+import org.junit.jupiter.api.extension.*;
+
+import java.util.List;
+import java.util.Optional;
+
+public class TomEEEmbeddedExtension extends ApplicationComposerExtensionBase implements BeforeAllCallback, AfterAllCallback, BeforeEachCallback, AfterEachCallback {
Review comment:
Good catch. `ApplicationComposerExtensionBase` isn't needed nor is it used.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tomee] rzo1 commented on a change in pull request #767: TOMEE-2977 - Provide a ApplicationComposer JUnit 5 Extension
Posted by GitBox <gi...@apache.org>.
rzo1 commented on a change in pull request #767:
URL: https://github.com/apache/tomee/pull/767#discussion_r592724306
##########
File path: container/openejb-core/src/main/java/org/apache/openejb/junit/jupiter/ApplicationComposerExtension.java
##########
@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit.jupiter;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.apache.openejb.testing.ApplicationComposers;
+import org.junit.jupiter.api.extension.*;
+
+public class ApplicationComposerExtension implements BeforeEachCallback, AfterEachCallback {
Review comment:
The current impl relies on the test's life cycle to start a container or perform related injections.
Atm the impl contains annotations and extensions for
- `RunWithSingleApplicationComposer`
- `RunWithApplicationComposer`
as well as a
- `TomEEEmbeddedExtension`
From my understanding, the proposed `mode` is solely responsible to configure how (or when) the underlying container is started? I.e. one container to run all tests within a single test class (PER_ALL), one container for each test method of one test class (PER_EACH) and one container to run multiple test classes (PER_JVM), right? Is this the same idea, which is currently conducted through different JUnit4 runners?
From my understanding of the current JUnit implementation, the `ApplicationComposer` seems to be something similar to the PER_ALL and the `EmbeddedTomEERunner` / `SingleApplicationComposer` to the PER_JVM; I think PER_EACH (test method) isn't implemented in the JUnit 4 runners yet.
Probably I am missing something but I think, I do not have the whole process or related use-cases in mind ;) - Maybe you , @rmannibucau , can add some more ideas / thoughts on the `mode` idea?
EDIT-1: I added some code to draft my idea / thoughts related to `modes`. Might be better to understand from the code point of view than from my words, if I got the idea correctly: https://github.com/rzo1/tomee/pull/4/files
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tomee] rzo1 commented on a change in pull request #767: TOMEE-2977 - Provide a ApplicationComposer JUnit 5 Extension
Posted by GitBox <gi...@apache.org>.
rzo1 commented on a change in pull request #767:
URL: https://github.com/apache/tomee/pull/767#discussion_r595489470
##########
File path: container/openejb-core/pom.xml
##########
@@ -580,6 +580,22 @@
<groupId>org.apache.xbean</groupId>
<artifactId>xbean-bundleutils</artifactId>
</dependency>
+ <dependency>
Review comment:
I think it would be (a) cleaner and (b) could allow user's to exclude the compile dependencies towards junit4 - so +1 for `openejb-junit5`.
Could also include the classes from `openejb-junit5-backward`, so we have all junit 5 related things in one module. wdyt?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tomee] rzo1 edited a comment on pull request #767: TOMEE-2977 - Provide a ApplicationComposer JUnit 5 Extension
Posted by GitBox <gi...@apache.org>.
rzo1 edited a comment on pull request #767:
URL: https://github.com/apache/tomee/pull/767#issuecomment-801920909
Should now contain the proposed fixes / enhancements.
Tests are now executed in an additional phase. Let's wait if we receive some additional feedback.
EDIT-1: Rebased the PR ([original commit history can be found here](https://github.com/rzo1/tomee/commits/2977-original-without-rebase)) - Jenkins PR builder was some sort of stuck. Hopefully it will now pick up the PR again.
--
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:
users@infra.apache.org
[GitHub] [tomee] rzo1 commented on a change in pull request #767: TOMEE-2977 - Provide a ApplicationComposer JUnit 5 Extension
Posted by GitBox <gi...@apache.org>.
rzo1 commented on a change in pull request #767:
URL: https://github.com/apache/tomee/pull/767#discussion_r596113257
##########
File path: container/openejb-core/src/main/java/org/apache/openejb/junit/jupiter/ApplicationComposerExtension.java
##########
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.junit.jupiter;
+
+import org.apache.openejb.OpenEJBRuntimeException;
+import org.apache.openejb.testing.ApplicationComposers;
+import org.apache.openejb.testing.SingleApplicationComposerBase;
+import org.junit.jupiter.api.extension.*;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+
+public class ApplicationComposerExtension extends ApplicationComposerExtensionBase implements BeforeAllCallback, AfterAllCallback, BeforeEachCallback, AfterEachCallback {
+
+ private static final ExtensionContext.Namespace NAMESPACE = ExtensionContext.Namespace.create(ApplicationComposerExtension.class.getName());
+ private static final SingleApplicationComposerBase BASE = new SingleApplicationComposerBase();
+
+ @Override
+ public void beforeAll(ExtensionContext context) throws Exception {
+
+ if (isPerJvm(context)) {
+ BASE.start(context.getTestClass().orElse(null));
Review comment:
Okay. I think I am understanding it now. Found some information about `-Dtomee.application-composer.application`, so I guess, we should do the validation.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tomee] rzo1 commented on a change in pull request #767: TOMEE-2977 - Provide a ApplicationComposer JUnit 5 Extension
Posted by GitBox <gi...@apache.org>.
rzo1 commented on a change in pull request #767:
URL: https://github.com/apache/tomee/pull/767#discussion_r596145338
##########
File path: container/openejb-core/pom.xml
##########
@@ -580,6 +580,22 @@
<groupId>org.apache.xbean</groupId>
<artifactId>xbean-bundleutils</artifactId>
</dependency>
+ <dependency>
Review comment:
Just moved everything into `openejb-junit5`.
Should we also include the contents of `openejb-junit5-backward` in this new `openejb-junit5` module? We haven't released it yet, so it wouldn't bother anyone.
Would bring a dependency towards the legacy testing stuff:
```
<dependency>
<groupId>${project.groupId}</groupId>
<artifactId>openejb-junit</artifactId>
<version>${project.version}</version>
</dependency>
```
Might be cleaner without it? wdyt @rmannibucau ?
----------------------------------------------------------------
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:
users@infra.apache.org