Ok think I get it. In this case there are few questions: why does the test enrich all apps? Shouldn't you declare some of them as not deployable?
Regarding the fix I'm still -1 on it - we used more or less the same to get WebBeansContext and that's pure bullshit and it breaks pretty easily - but +1 to check that's the class we want with the classloader. wdyt? Can you validate it is enough for your case? Romain Manni-Bucau Twitter: @rmannibucau Blog: http://rmannibucau.wordpress.com/ LinkedIn: http://fr.linkedin.com/in/rmannibucau Github: https://github.com/rmannibucau 2014-09-23 19:38 GMT+02:00 Andy Gumbrecht <[email protected]>: > 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 >
