The tests work fine in Java 8 and fail in Java 7, so it's not even close to say work only on the test? The issue may be related to the test, but it is not the test itself.

The sort algorithm does not rely on CDI, that is just 'one' of 'five' rankings used to determine the order.

The test declares several app deployments. The ContainerSystem deployments list the following BeanContexts, which looks right - but as you say may be wrong (and should be the focus if it is):

yellow_com.fqn.ApplicationApiTest
blue_com.fqn.ApplicationApiTest
orange_com.fqn.ApplicationApiTest
green_com.fqn.ApplicationApiTest

We are looking for 'orange' because that is the actual test app. So if we provide 'appname_ + className' as the single criteria to locate this AppContext, and only an iterator to find it then we are just using luck.

On Java 8 we just get lucky that 'orange_com.fqn.ApplicationApiTest' is first in the list, on Java 7 it's 'green_com.fqn.ApplicationApiTest', so the tests clearly fail.

If there is only one AppContext then it is 'highly' likely to be the test AppContext for the provided class, else something has gone seriously wrong? - You could still check the BeanContexts if you really feel that is worth it.

We can revert it if you provide a better solution, which I am sure you will, then please do. But not until then, as the fix currently solves the issue.

Andy.

On 23/09/2014 15:02, Romain Manni-Bucau wrote:
Hmm

seems wrong:

1) if a single app you return it -> that's not what we want most of
the time and will lead to the issue you reported
2) sort algorithm relies on CDI which is not at all what we want since
we support not cdi enrichment as well

I'd revert it and work on the test first - well I'll do if you don't ;).

Idea is to be able to identify the bean AppContext. ClassLoader can
help (we have an utility for it).

If classloader is not enough for it then you need to enrich with all
appcontexts but I guess it can be a config issue then.


Romain Manni-Bucau
Twitter: @rmannibucau
Blog: http://rmannibucau.wordpress.com/
LinkedIn: http://fr.linkedin.com/in/rmannibucau
Github: https://github.com/rmannibucau



---------- Forwarded message ----------
From:  <[email protected]>
Date: 2014-09-23 14:51 GMT+02:00
Subject: svn commit: r1626990 -
/tomee/tomee/branches/tomee-1.7.x/arquillian/arquillian-tomee-common/src/main/java/org/apache/openejb/arquillian/common/TomEEInjectionEnricher.java
To: [email protected]


Author: andygumbrecht
Date: Tue Sep 23 12:51:46 2014
New Revision: 1626990

URL: http://svn.apache.org/r1626990
Log:
TOMEE-1359 - Returns a better resolution if found multiple times.
Works for known failing tests in tribestream
This is for review - Test case in TomEE to follow.

Modified:
     
tomee/tomee/branches/tomee-1.7.x/arquillian/arquillian-tomee-common/src/main/java/org/apache/openejb/arquillian/common/TomEEInjectionEnricher.java

Modified: 
tomee/tomee/branches/tomee-1.7.x/arquillian/arquillian-tomee-common/src/main/java/org/apache/openejb/arquillian/common/TomEEInjectionEnricher.java
URL: 
http://svn.apache.org/viewvc/tomee/tomee/branches/tomee-1.7.x/arquillian/arquillian-tomee-common/src/main/java/org/apache/openejb/arquillian/common/TomEEInjectionEnricher.java?rev=1626990&r1=1626989&r2=1626990&view=diff
==============================================================================
--- 
tomee/tomee/branches/tomee-1.7.x/arquillian/arquillian-tomee-common/src/main/java/org/apache/openejb/arquillian/common/TomEEInjectionEnricher.java
(original)
+++ 
tomee/tomee/branches/tomee-1.7.x/arquillian/arquillian-tomee-common/src/main/java/org/apache/openejb/arquillian/common/TomEEInjectionEnricher.java
Tue Sep 23 12:51:46 2014
@@ -28,6 +28,10 @@ import org.jboss.arquillian.test.spi.Tes
  import org.jboss.arquillian.test.spi.TestEnricher;

  import java.lang.reflect.Method;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.List;
  import java.util.logging.Level;
  import java.util.logging.Logger;

@@ -46,13 +50,90 @@ public class TomEEInjectionEnricher impl

      private AppContext getAppContext(final String className) {
          final ContainerSystem containerSystem =
SystemInstance.get().getComponent(ContainerSystem.class);
-        for (final AppContext app : containerSystem.getAppContexts()) {
+        final List<AppContext> appContexts = containerSystem.getAppContexts();
+
+        final int size = appContexts.size();
+        if (size == 1) {
+            return appContexts.get(0);
+        }
+
+        final List<AppContext> found = new ArrayList<AppContext>(size);
+
+        for (final AppContext app : appContexts) {
              final BeanContext context =
containerSystem.getBeanContext(app.getId() + "_" + className);
              if (context != null) {
-                return app;
+                found.add(app);
              }
          }

+        if (found.size() > 0) {
+
+            Collections.sort(found, new Comparator<AppContext>() {
+
+                /**
+                 * If multiple apps are found that contain the test
class then a best guess effort needs to be made
+                 * to find the context that best matches the test
class application.
+                 *
+                 * @param ac1 AppContext
+                 * @param ac2 AppContext
+                 * @return int
+                 */
+                @Override
+                public int compare(final AppContext ac1, final
AppContext ac2) {
+                    int c = 0;
+
+                    if (isBeanManagerInUse(ac1) && !isBeanManagerInUse(ac2)) {
+                        c--;
+                    } else if (!isBeanManagerInUse(ac1) &&
isBeanManagerInUse(ac2)) {
+                        c++;
+                    }
+
+                    if (ac1.isCdiEnabled() && !ac2.isCdiEnabled()) {
+                        c--;
+                    } else if (!ac1.isCdiEnabled() && ac2.isCdiEnabled()) {
+                        c++;
+                    }
+
+                    int size1 = ac1.getBeanContexts().size();
+                    int size2 = ac2.getBeanContexts().size();
+                    if (size1 > size2) {
+                        c--;
+                    } else if (size2 > size1) {
+                        c++;
+                    }
+
+                    size1 = ac1.getBindings().size();
+                    size2 = ac2.getBindings().size();
+                    if (size1 > size2) {
+                        c--;
+                    } else if (size2 > size1) {
+                        c++;
+                    }
+
+                    size1 = ac1.getWebContexts().size();
+                    size2 = ac2.getWebContexts().size();
+                    if (size1 > size2) {
+                        c--;
+                    } else if (size2 > size1) {
+                        c++;
+                    }
+
+                    return c;
+                }
+
+                private boolean isBeanManagerInUse(final AppContext ac) {
+                    try {
+                        return
ac.getWebBeansContext().getBeanManagerImpl().isInUse();
+                    } catch (final Exception e) {
+                        return false;
+                    }
+                }
+            });
+
+            //Return the most likely candidate
+            return found.get(0);
+        }
+
          
Logger.getLogger(TomEEInjectionEnricher.class.getName()).log(Level.WARNING,
"Failed to find AppContext for: " + className);

          return null;




--
  Andy Gumbrecht
  https://twitter.com/AndyGeeDe
  http://www.tomitribe.com

Reply via email to