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: [email protected]
