Improve error handling in the Composite Service Context
The extender does no more hold the lock to start the management of already installed bundles (Felix-985)
Improve the maven-junit4osgi-plugin: Report are now in the surefire-report folder, possibility to enable/disable System.out and System.err, allow configuring Felix.


git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@756042 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/ipojo/composite/src/main/java/org/apache/felix/ipojo/composite/CompositeServiceContext.java b/ipojo/composite/src/main/java/org/apache/felix/ipojo/composite/CompositeServiceContext.java
index 1b0ebfc..5bdc75e 100644
--- a/ipojo/composite/src/main/java/org/apache/felix/ipojo/composite/CompositeServiceContext.java
+++ b/ipojo/composite/src/main/java/org/apache/felix/ipojo/composite/CompositeServiceContext.java
@@ -29,6 +29,7 @@
 import org.apache.felix.ipojo.Factory;

 import org.apache.felix.ipojo.IPojoContext;

 import org.apache.felix.ipojo.ServiceContext;

+import org.apache.felix.ipojo.context.ServiceReferenceImpl;

 import org.apache.felix.ipojo.context.ServiceRegistry;

 import org.apache.felix.ipojo.util.Tracker;

 import org.apache.felix.ipojo.util.TrackerCustomizer;

@@ -160,7 +161,11 @@
      * @see org.apache.felix.ipojo.ServiceContext#getService(org.osgi.framework.ServiceReference)

      */

     public Object getService(ServiceReference arg0) {

-        return m_registry.getService(m_instance, arg0);

+        if (arg0 instanceof ServiceReferenceImpl) {

+            return m_registry.getService(m_instance, arg0);

+        } else {

+            throw new RuntimeException("Cannot get a global service from the local registry");

+        }

     }

 

     

diff --git a/ipojo/core/src/main/java/org/apache/felix/ipojo/Extender.java b/ipojo/core/src/main/java/org/apache/felix/ipojo/Extender.java
index 0b7b350..517b888 100644
--- a/ipojo/core/src/main/java/org/apache/felix/ipojo/Extender.java
+++ b/ipojo/core/src/main/java/org/apache/felix/ipojo/Extender.java
@@ -126,7 +126,7 @@
                 break;

             case BundleEvent.STOPPING:

                 m_thread.removeBundle(event.getBundle());

-                closeManagementFor(event.getBundle());

+                closeManagementFor(event.getBundle()); //TODO Should be done in another thread

                 break;

             default:

                 break;

@@ -300,7 +300,7 @@
             // compute already started bundles.

             for (int i = 0; i < context.getBundles().length; i++) {

                 if (context.getBundles()[i].getState() == Bundle.ACTIVE) {

-                    startManagementFor(context.getBundles()[i]);

+                    m_thread.addBundle(context.getBundles()[i]); // Bundles are processed in another thread.

                 }

             }

         }

diff --git a/ipojo/core/src/main/java/org/apache/felix/ipojo/MethodInterceptor.java b/ipojo/core/src/main/java/org/apache/felix/ipojo/MethodInterceptor.java
index 854405f..c74a11c 100644
--- a/ipojo/core/src/main/java/org/apache/felix/ipojo/MethodInterceptor.java
+++ b/ipojo/core/src/main/java/org/apache/felix/ipojo/MethodInterceptor.java
@@ -34,7 +34,8 @@
 public interface MethodInterceptor {

     

     /**

-     * This method is called when the execution enters in a method.

+     * This method is called when a thread enters in a method.

+     * The given argument array is created from the method argument.

      * @param pojo the pojo on which the method is called.

      * @param method the invoked method.

      * @param args the arguments array.

@@ -46,7 +47,7 @@
      * before a <code>return</code>.

      * If the given returned object is <code>null</code>, either the method is 

      * <code>void</code>, or it returns <code>null</code>.

-     * You must not modified the returned object.

+     * This method must not modify the returned object.

      * @param pojo the pojo on which the method exits.

      * @param method the exiting method.

      * @param returnedObj the the returned object (boxed for primitive type)

diff --git a/ipojo/junit4osgi/maven-junit4osgi-plugin/src/main/java/org/apache/felix/ipojo/junit4osgi/plugin/Junit4osgiPlugin.java b/ipojo/junit4osgi/maven-junit4osgi-plugin/src/main/java/org/apache/felix/ipojo/junit4osgi/plugin/Junit4osgiPlugin.java
index c064c2b..c32af8c 100644
--- a/ipojo/junit4osgi/maven-junit4osgi-plugin/src/main/java/org/apache/felix/ipojo/junit4osgi/plugin/Junit4osgiPlugin.java
+++ b/ipojo/junit4osgi/maven-junit4osgi-plugin/src/main/java/org/apache/felix/ipojo/junit4osgi/plugin/Junit4osgiPlugin.java
@@ -78,7 +78,7 @@
     /**
      * Base directory where all reports are written to.
      * 
-     * @parameter expression="${project.build.directory}/junit4osgi-reports"
+     * @parameter expression="${project.build.directory}/surefire-reports"
      */
     private File m_reportsDirectory;
     
@@ -99,9 +99,16 @@
     /**
      * Required bundles.
      * 
-     * @parameter expression="${bundles}"
+     * @parameter
      */
-    private ArrayList m_bundles;
+    private List bundles;
+    
+    /**
+     * Felix configuration.
+     * 
+     * @parameter
+     */
+    private Map configuration;
     
     /**
      * Enables / Disables the log service provided by the plugin.
@@ -145,6 +152,35 @@
      */
     private LogServiceImpl m_logService;
     
+    /**
+     * Set this to 'true' to bypass unit tests entirely. Its use is NOT RECOMMENDED, especially if you
+     * enable it using the "maven.test.skip" property, because maven.test.skip disables both running the
+     * tests and compiling the tests.  Consider using the skipTests parameter instead.
+     * 
+     * @parameter expression="${maven.test.skip}"
+     */
+    private boolean skip;
+    
+    /**
+     * Set this to true to ignore a failure during testing. Its use is NOT RECOMMENDED, but quite convenient on
+     * occasion.
+     * 
+     * @parameter expression="${maven.test.failure.ignore}"
+     */
+    private boolean testFailureIgnore;
+    
+    /**
+     * Set this to avoid printing test execution trace on System.out and System.err. This will be written in the
+     * reports.
+     * @parameter 
+     */
+    private boolean hideOutputs;
+    
+    /**
+     * Felix configuration.
+     */
+    private Map felixConf;
+    
    
     /**
      * Executes the plug-in.
@@ -153,6 +189,12 @@
      */
     public void execute() throws MojoFailureException {
         
+        if (skip) {
+            getLog().info("Tests are skipped");
+            return;
+        }
+       
+        
         List bundles = parseBundleList();
         bundles.addAll(getTestBundle());
         
@@ -164,24 +206,40 @@
             getLog().info("Log Service disabled");
         }
         activators.add(new Installer(m_pluginArtifacts, bundles, m_project, m_deployProjectArtifact));
-        Map map = new HashMap();
-        map.put("felix.systembundle.activators", activators);
-        map.put("org.osgi.framework.storage.clean", "onFirstInit");
-        map.put("ipojo.log.level", "WARNING");
+        felixConf = new HashMap();
+        felixConf.put("felix.systembundle.activators", activators);
+        felixConf.put("org.osgi.framework.storage.clean", "onFirstInit");
+        felixConf.put("ipojo.log.level", "WARNING");
         // Use a boot delagation to share classes between the host and the embedded Felix.
         // The junit.framework package is boot delegated to execute tests
         // The log service package is also boot delegated as the host publish a log service
         // The cobertura package is used during code coverage collection
-        map.put("org.osgi.framework.bootdelegation", "junit.framework, org.osgi.service.log, net.sourceforge.cobertura.coveragedata"); 
+        felixConf.put("org.osgi.framework.bootdelegation", "junit.framework, org.osgi.service.log, net.sourceforge.cobertura.coveragedata"); 
         
-        map.put("org.osgi.framework.storage", m_targetDir.getAbsolutePath() + "/felix-cache"); 
+        felixConf.put("org.osgi.framework.storage", m_targetDir.getAbsolutePath() + "/felix-cache"); 
+        
+        
+        if (configuration != null) {
+            felixConf.putAll(configuration);
+            // Check boot delegation
+            String bd = (String) felixConf.get("org.osgi.framework.bootdelegation");
+            if (bd.indexOf("junit.framework") == -1) {
+                bd.concat(", junit.framework");
+            }
+            if (bd.indexOf("org.osgi.service.log") == -1) {
+                bd.concat(", org.osgi.service.log");
+            }
+            if (bd.indexOf("net.sourceforge.cobertura.coveragedata") == -1) {
+                bd.concat(", net.sourceforge.cobertura.coveragedata");
+            }
+        }
         
         System.out.println("");
         System.out.println("-------------------------------------------------------");
         System.out.println(" T E S T S");        
         System.out.println("-------------------------------------------------------");
         
-        Felix felix = new Felix(map);
+        Felix felix = new Felix(felixConf);
         try {
             felix.start();
         } catch (BundleException e) {
@@ -197,14 +255,23 @@
         try {
             felix.stop();
             felix.waitForStop(5000);
+            // Delete felix-cache
+            File cache = new File(m_targetDir.getAbsolutePath() + "/felix-cache");
+            cache.delete();
         } catch (Exception e) {
             getLog().error(e);
         }
         
         if (m_totalErrors > 0 || m_totalFailures > 0) {
+            if (! testFailureIgnore) {
             throw new MojoFailureException("There are test failures. \n\n"
                     + "Please refer to " + m_reportsDirectory.getAbsolutePath()
                     + " for the individual test results.");
+            } else {
+                getLog().warn("There are test failures. \n\n"
+                    + "Please refer to " + m_reportsDirectory.getAbsolutePath()
+                    + " for the individual test results.");
+            }
         }
 
     }
@@ -289,12 +356,12 @@
      */
     private List parseBundleList() {
         List toDeploy = new ArrayList();
-        if (m_bundles == null) {
+        if (bundles == null) {
             return toDeploy;
         }
-        
-        for (int i = 0; i < m_bundles.size(); i++) {
-            String bundle = (String) m_bundles.get(i);
+        System.out.println("Deploy URL bundles " + bundles); // TODO
+        for (int i = 0; i < bundles.size(); i++) {
+            String bundle = (String) bundles.get(i);
             try {
                 URL url = new URL(bundle);
                 toDeploy.add(url);
@@ -317,7 +384,7 @@
         Set dependencies = m_project.getDependencyArtifacts();
         for (Iterator artifactIterator = dependencies.iterator(); artifactIterator.hasNext();) {
             Artifact artifact = (Artifact) artifactIterator.next();
-            if (Artifact.SCOPE_TEST.equals(artifact.getScope())) { // Select scope=test.
+            if (artifact.getScope() != null) { // Select not null scope... [Select every bundles with a scope TEST, COMPILE and RUNTIME]
                 File file = artifact.getFile();
                 try {
                     if (file.exists()) {
@@ -326,11 +393,11 @@
                             if (jar.getManifest().getMainAttributes().getValue("Bundle-ManifestVersion") != null) {
                                 toDeploy.add(file.toURL());
                             }
-                        } else {
-                            getLog().info("The test artifact " + artifact.getFile().getName() + " is not a Jar file.");
-                        }
+                        } // else {
+//                            getLog().info("The artifact " + artifact.getFile().getName() + " is not a Jar file.");
+//                        }
                     } else {
-                        getLog().info("The test artifact " + artifact.getFile().getName() + " does not exist.");
+                        getLog().info("The artifact " + artifact.getFile().getName() + " does not exist.");
                     }
                 } catch (Exception e) {
                     getLog().error(file + " is not a valid bundle, this artifact is ignored");
@@ -503,7 +570,7 @@
             m_totalFailures += tr.failureCount();
             m_totalErrors += tr.errorCount();
 
-            report.generateReport(test, tr, m_reportsDirectory, bc);
+            report.generateReport(test, tr, m_reportsDirectory, bc, felixConf);
 
         } catch (Exception e) {
             getLog().error(e);
@@ -615,8 +682,8 @@
         public void startTest(Test test) {
             m_abort = false;
             m_report.testStarting();
-            System.setErr(new PrintStream(m_err));
-            System.setOut(new PrintStream(m_out));
+            System.setErr(new ReportPrintStream(m_err,m_errBackup, hideOutputs));
+            System.setOut(new ReportPrintStream(m_out, m_outBackup, hideOutputs));
             getLogService().enableOutputStream();
         }
         
diff --git a/ipojo/junit4osgi/maven-junit4osgi-plugin/src/main/java/org/apache/felix/ipojo/junit4osgi/plugin/ReportPrintStream.java b/ipojo/junit4osgi/maven-junit4osgi-plugin/src/main/java/org/apache/felix/ipojo/junit4osgi/plugin/ReportPrintStream.java
new file mode 100644
index 0000000..97e8fd3
--- /dev/null
+++ b/ipojo/junit4osgi/maven-junit4osgi-plugin/src/main/java/org/apache/felix/ipojo/junit4osgi/plugin/ReportPrintStream.java
@@ -0,0 +1,101 @@
+/* 
+ * 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.felix.ipojo.junit4osgi.plugin;
+
+import java.io.OutputStream;
+import java.io.PrintStream;
+
+/**
+ * Print stream dispatching on a given one and storing written data
+ * in a output stream.
+ * @author <a href="mailto:dev@felix.apache.org">Felix Project Team</a>
+ */
+public class ReportPrintStream extends PrintStream {
+
+    private PrintStream m_stream;
+    
+    private boolean m_duplicate;
+
+    public ReportPrintStream(OutputStream out, PrintStream def, boolean hideOutput) {
+        super(out);
+        m_stream = def;
+        m_duplicate = ! hideOutput;
+    }
+
+    public void println() {
+        if (m_duplicate) { m_stream.println(); }
+        super.println();
+    }
+
+
+    public void println(boolean x) {
+        if (m_duplicate) { m_stream.println(x); }
+        super.println(x);
+    }
+
+
+    public void println(char x) {
+        if (m_duplicate) { m_stream.println(x); }
+        super.println(x);
+    }
+
+
+    public void println(char[] x) {
+        if (m_duplicate) { m_stream.println(x); }
+        super.println(x);
+    }
+
+
+    public void println(double x) {
+        if (m_duplicate) { m_stream.println(x); }
+        super.println(x);
+    }
+
+
+    public void println(float x) {
+        if (m_duplicate) { m_stream.println(x); }
+        super.println(x);
+    }
+
+
+    public void println(int x) {
+        if (m_duplicate) { m_stream.println(x); }
+        super.println(x);
+    }
+
+
+
+    public void println(long x) {
+        if (m_duplicate) { m_stream.println(x); }
+        super.println(x);
+    }
+
+
+    public void println(Object x) {
+        if (m_duplicate) { m_stream.println(x); }
+        super.println(x);
+    }
+
+
+    public void println(String s) {
+        if (m_duplicate) { m_stream.println(s); }
+        super.println(s);
+    }
+
+}
diff --git a/ipojo/junit4osgi/maven-junit4osgi-plugin/src/main/java/org/apache/felix/ipojo/junit4osgi/plugin/XMLReport.java b/ipojo/junit4osgi/maven-junit4osgi-plugin/src/main/java/org/apache/felix/ipojo/junit4osgi/plugin/XMLReport.java
index bc8e6ac..14cad80 100644
--- a/ipojo/junit4osgi/maven-junit4osgi-plugin/src/main/java/org/apache/felix/ipojo/junit4osgi/plugin/XMLReport.java
+++ b/ipojo/junit4osgi/maven-junit4osgi-plugin/src/main/java/org/apache/felix/ipojo/junit4osgi/plugin/XMLReport.java
@@ -29,6 +29,7 @@
 import java.util.Enumeration;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Map;
 import java.util.Properties;
 import java.util.StringTokenizer;
 
@@ -153,15 +154,16 @@
      * @param tr the test result
      * @param reportsDirectory the directory in which reports are created.
      * @param bc the bundle context (to get installed bundles)
+     * @param configuration the Felix configuration
      * @throws Exception when the XML report cannot be generated correctly
      */
     public void generateReport(Test test, TestResult tr, File reportsDirectory,
-            BundleContext bc) throws Exception {
+            BundleContext bc, Map configuration) throws Exception {
         long runTime = this.m_endTime - this.m_startTime;
 
         Xpp3Dom testSuite = createTestSuiteElement(test, runTime);
 
-        showProperties(testSuite, bc);
+        showProperties(testSuite, bc, configuration);
 
         testSuite.setAttribute("tests", String.valueOf(tr.runCount()));
 
@@ -258,10 +260,11 @@
      * This method also adds installed bundles.
      * @param testSuite the XML element.
      * @param bc the bundle context
+     * @param configuration the configuration of the underlying OSGi platform
      */
-    private void showProperties(Xpp3Dom testSuite, BundleContext bc) {
+    private void showProperties(Xpp3Dom testSuite, BundleContext bc, Map configuration) {
         Xpp3Dom properties = createElement(testSuite, "properties");
-
+        
         Properties systemProperties = System.getProperties();
 
         if (systemProperties != null) {
@@ -284,6 +287,31 @@
 
             }
         }
+        
+        if (configuration != null) {
+            Iterator it = configuration.keySet().iterator();
+
+            while (it.hasNext()) {
+                String key = (String) it.next();
+
+                Object obj = (Object) configuration.get(key);
+                String value = null;
+                if (obj == null) {
+                    value = "null";
+                } else if (obj instanceof String) {
+                    value = (String) obj;
+                } else {
+                    value  = obj.toString();
+                }
+
+                Xpp3Dom property = createElement(properties, "property");
+
+                property.setAttribute("name", key);
+
+                property.setAttribute("value", value);
+
+            }
+        }
 
         Xpp3Dom buns = createElement(properties, "bundles");
         Bundle[] bundles = bc.getBundles();
diff --git a/ipojo/tests/integration-tests/pom.xml b/ipojo/tests/integration-tests/pom.xml
index 706b491..4218abe 100644
--- a/ipojo/tests/integration-tests/pom.xml
+++ b/ipojo/tests/integration-tests/pom.xml
@@ -22,10 +22,15 @@
 						<goals>
 							<goal>test</goal>
 						</goals>
+						<configuration>
+							<hideOutputs>true</hideOutputs>
+							<configuration>
+							<org.osgi.http.port>8083</org.osgi.http.port>
+							</configuration>
+						</configuration>
 					</execution>
 				</executions>
 			</plugin>
-			
 		</plugins>
 	</build>
 	<reporting>