ingwarsw / arquillian-suite-extension

Arquillian extension that will force single deployment for all tests
Apache License 2.0
67 stars 20 forks source link

Bug? Duplicated class annotated with ArquillianS.. #25

Closed phlbrz closed 9 years ago

phlbrz commented 9 years ago

Hello, Ingwar!

Is Duplicated class annotated with @ArquillianSuiteDeployment a bug? Fixing a bug when inside eclipse you open two or more projects that are dependency with other and it shows that the @ArquillianSuiteDeployment is duplicated. Maybe it happens on maven test also. Example, can be project or module: Project A Project B Project A - Has a Test Class ExampleProjectATestSuite with @ArquillianSuiteDeployment and @Deployment Project A - Has Test Classes extending ExampleProjectATestSuite Project B - Depends on Project A Project B - Has your own Test Class ExampleProjectBTestSuite with @ArquillianSuiteDeployment and @Deployment Project B - Has Test Classes extending ExampleProjectBTestSuite Project A - mvn test or run test inside eclipse Project A - SUCCESS; Project B - mvn test or run test inside eclipse. Project B - SEVERE ERROR: log.log(Level.SEVERE, "arquillian-suite-deployment: Duplicated class annotated with @ArquillianSuiteDeployment: {0}", type.getName());

Is it normal? Because using that code that I'm pushing solved my problem and my projects runs smoothly and at light speed.

I found another problem that when the application is undeploying, but not the datasource, when the second project is running, says that the datasource already exists. But it can be another change, I'll search something. If it's not the right place to commit, sorry, just say to me where I should commit.

Thank you!

ingwarsw commented 9 years ago

Thanks.. I suppose now its scanning project own classpath not all possible ones..

phlbrz commented 9 years ago

Yes! Thank you!

phlbrz commented 9 years ago

Are you going to generate a new snapshot soon?

ingwarsw commented 9 years ago

I will try to create one in next week..

phlbrz commented 9 years ago

Just a feedback. With this patch, my tests are running fast with Java EE 6 (CDI+EJB+JPA) and Wicket than before with Spring and with less configurations. I have more tests to fix in my application but it's a great start. screenshot from 2015-06-22 21 11 49

Thank's again.

Adrodoc commented 6 years ago

This bug fix breaks arquillian suite extension for me (in eclipse), I suggest reverting it or using a different approach. The issue is, that right now you scan the first classpath root, which depends on the ordering of your classpath, which is highly undesireable in my opinion. For instance: if you use Gradle to configure your Eclipse, then you have seperated bin folders and the main bin folder is the first classpath root. In this case the annotated class containing the deployment is not found, because it is in the test bin folder (or insituTest bin folder in my case). One way to improve the situation is to use the current implementation first and if there is no match you scan the whole classpath like before 3f2a15f. This way the problem of @phlbrz is still solved, but people who use different bin folder/classpath roots can use arquillian suite extension too. This would be a very easy fix.

Current Implementation:

    // Had a bug that if you open inside eclipse more than one project with @ArquillianSuiteDeployment and is a
    // dependency, the test doesn't run because found more than one @ArquillianSuiteDeployment.
    // Filter the deployment PER project.
    final Reflections reflections = new Reflections(ClasspathHelper.contextClassLoader().getResource(""));
    // Reflection all classes to search for Annotation @ArquillianSuiteDeployment.
    Set<Class<?>> results = reflections.getTypesAnnotatedWith(ArquillianSuiteDeployment.class, true);
    if (results.isEmpty()) {
      // Keeping compatibility backward.
      results = reflections.getTypesAnnotatedWith(ArquilianSuiteDeployment.class, true);
      if (results.isEmpty()) {
        return null;
      }
    }

Proposed fix:

    // Search for deployment class in first classpath entry (filter the deployment PER project in eclipse)
    final Reflections reflections = new Reflections(ClasspathHelper.contextClassLoader().getResource(""));
    Set<Class<?>> results = reflections.getTypesAnnotatedWith(ArquillianSuiteDeployment.class, true);
    if (results.isEmpty()) {
      // Search for deployment class in whole classpath
      results = new Reflections("").getTypesAnnotatedWith(ArquillianSuiteDeployment.class, true);
      if (results.isEmpty()) {
        // Keeping compatibility backward.
        results = reflections.getTypesAnnotatedWith(ArquilianSuiteDeployment.class, true);
        if (results.isEmpty()) {
          return null;
        }
      }
    }

Note that with this fix you still can't use @ArquillianSuiteDeployment if you have a project dependency as described by @phlbrz AND you use seperate bin folders as generated by Gradle at the same time. I guess for this case it is better to not throw an IllegalStateException here and return null instead. This way you still get warned in the log that arquillian-suite-deployment is not active, but you can at least execute the tests in eclipse.

ingwarsw commented 6 years ago

Could you try using version from branch feature/configure_by_xml and configure it by xml? I didnt test it well.. but I suppose thats better approach..

Adrodoc commented 6 years ago

@ingwarsw no i didn't, is there a jar file that I can use or would I have to build one from source?

ingwarsw commented 6 years ago

You would have to build it...

Adrodoc commented 6 years ago

@ingwarsw Unfortunately I don't have maven installed and I have never used it, so I am not sure if I could do that :)

ingwarsw commented 6 years ago

mvn install but maven is surely needed :)

Adrodoc commented 6 years ago

@ingwarsw i managed to build a local snapshot, where can i find documentation on how to configure this with xml?

ingwarsw commented 6 years ago

So far only in test code..

<extension qualifier="suite">
    <property name="deploymentClass">org.eu.ingwar.tools.arquillian.extension.suite.Deployments</property>
</extension>

https://github.com/ingwarsw/arquillian-suite-extension/commit/8e8caea96994d87c229788bcb9dfa5d1a3156a15#diff-315a84da27aa304bb14c9f7e7a5642beR24

Adrodoc commented 6 years ago

@ingwarsw your example was missing a >, but other than that the xml variant works too. I still think you should merge #48

Adrodoc commented 6 years ago

@ingwarsw Do you have a rough idea when you'll release a version with #48 or feature/configure_by_xml? I'd really like to upgrade from version 1.1.2, because I want to use arquillian 1.2.0 and currently we are stuck at 1.1.12.

ingwarsw commented 6 years ago

Will try to do that today..