You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jmeter.apache.org by pm...@apache.org on 2016/01/28 23:23:24 UTC

svn commit: r1727442 - in /jmeter/trunk: src/jorphan/org/apache/jorphan/reflect/ test/src/org/apache/jmeter/gui/util/ test/src/org/apache/jmeter/junit/ test/src/org/apache/jorphan/test/ xdocs/

Author: pmouawad
Date: Thu Jan 28 22:23:23 2016
New Revision: 1727442

URL: http://svn.apache.org/viewvc?rev=1727442&view=rev
Log:
Bug 58897 : Improve JUnit Test code
#resolve #85
Bugzilla Id: 58897

Added:
    jmeter/trunk/src/jorphan/org/apache/jorphan/reflect/ClassFilter.java   (with props)
Modified:
    jmeter/trunk/src/jorphan/org/apache/jorphan/reflect/ClassFinder.java
    jmeter/trunk/test/src/org/apache/jmeter/gui/util/TestMenuFactory.java
    jmeter/trunk/test/src/org/apache/jmeter/junit/JMeterTest.java
    jmeter/trunk/test/src/org/apache/jorphan/test/AllTests.java
    jmeter/trunk/xdocs/changes.xml

Added: jmeter/trunk/src/jorphan/org/apache/jorphan/reflect/ClassFilter.java
URL: http://svn.apache.org/viewvc/jmeter/trunk/src/jorphan/org/apache/jorphan/reflect/ClassFilter.java?rev=1727442&view=auto
==============================================================================
--- jmeter/trunk/src/jorphan/org/apache/jorphan/reflect/ClassFilter.java (added)
+++ jmeter/trunk/src/jorphan/org/apache/jorphan/reflect/ClassFilter.java Thu Jan 28 22:23:23 2016
@@ -0,0 +1,32 @@
+/*
+ * 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.jorphan.reflect;
+
+/**
+ * filter class when visiting the search path with {@link ClassFinder}
+ * @since 3.0
+ */
+public interface ClassFilter {
+
+    /**
+     * @param className String class name
+     * @return true if class is included
+     */
+    boolean accept(String className);
+}

Propchange: jmeter/trunk/src/jorphan/org/apache/jorphan/reflect/ClassFilter.java
------------------------------------------------------------------------------
    svn:mime-type = text/plain

Modified: jmeter/trunk/src/jorphan/org/apache/jorphan/reflect/ClassFinder.java
URL: http://svn.apache.org/viewvc/jmeter/trunk/src/jorphan/org/apache/jorphan/reflect/ClassFinder.java?rev=1727442&r1=1727441&r2=1727442&view=diff
==============================================================================
--- jmeter/trunk/src/jorphan/org/apache/jorphan/reflect/ClassFinder.java (original)
+++ jmeter/trunk/src/jorphan/org/apache/jorphan/reflect/ClassFinder.java Thu Jan 28 22:23:23 2016
@@ -56,13 +56,10 @@ public final class ClassFinder {
     }
 
     /**
-     * Filter updates to TreeSet by only storing classes
+     * Filter updates by only storing classes
      * that extend one of the parent classes
-     *
-     *
      */
-    private static class FilterTreeSet extends TreeSet<String>{
-        private static final long serialVersionUID = 234L;
+    private static class ExtendsClassFilter implements ClassFilter {
 
         private final Class<?>[] parents; // parent classes to check
         private final boolean inner; // are inner classes OK?
@@ -75,73 +72,50 @@ public final class ClassFinder {
         private final transient ClassLoader contextClassLoader
             = Thread.currentThread().getContextClassLoader(); // Potentially expensive; do it once
 
-        FilterTreeSet(Class<?> []parents, boolean inner, String contains, String notContains){
-            super();
-            this.parents=parents;
-            this.inner=inner;
-            this.contains=contains;
-            this.notContains=notContains;
-        }
-
-        /**
-         * Override the superclass so we only add classnames that
-         * meet the criteria.
-         *
-         * @param s - classname (must be a String)
-         * @return true if it is a new entry
-         *
-         * @see java.util.TreeSet#add(java.lang.Object)
-         */
+        ExtendsClassFilter(Class<?> []parents, boolean inner, String contains, String notContains){
+            this.parents = parents;
+            this.inner = inner;
+            this.contains = contains;
+            this.notContains = notContains;
+        }
+
         @Override
-        public boolean add(String s){
-            if (contains(s)) {
-                return false;// No need to check it again
-            }
-            if (contains!=null && s.indexOf(contains) == -1){
+        public boolean accept(String className) {
+            
+            if (contains!=null && className.indexOf(contains) == -1){
                 return false; // It does not contain a required string
             }
-            if (notContains!=null && s.indexOf(notContains) != -1){
+            if (notContains!=null && className.indexOf(notContains) != -1){
                 return false; // It contains a banned string
             }
-            if ((s.indexOf('$') == -1) || inner) { // $NON-NLS-1$
-                if (isChildOf(parents,s, contextClassLoader)) {
-                    return super.add(s);
+            if ((className.indexOf('$') == -1) || inner) { // $NON-NLS-1$
+                if (isChildOf(parents,className, contextClassLoader)) {
+                    return true;
                 }
             }
             return false;
         }
     }
 
-    private static class AnnoFilterTreeSet extends TreeSet<String>{
-        private static final long serialVersionUID = 240L;
-
+    
+    private static class AnnoClassFilter implements ClassFilter {
+        
         private final boolean inner; // are inner classes OK?
 
         private final Class<? extends Annotation>[] annotations; // annotation classes to check
         private final transient ClassLoader contextClassLoader
             = Thread.currentThread().getContextClassLoader(); // Potentially expensive; do it once
-        AnnoFilterTreeSet(Class<? extends Annotation> []annotations, boolean inner){
-            super();
+        
+        AnnoClassFilter(Class<? extends Annotation> []annotations, boolean inner){
             this.annotations = annotations;
-            this.inner=inner;
+            this.inner = inner;
         }
-        /**
-         * Override the superclass so we only add classnames that
-         * meet the criteria.
-         *
-         * @param s - classname (must be a String)
-         * @return true if it is a new entry
-         *
-         * @see java.util.TreeSet#add(java.lang.Object)
-         */
+        
         @Override
-        public boolean add(String s){
-            if (contains(s)) {
-                return false;// No need to check it again
-            }
-            if ((s.indexOf('$') == -1) || inner) { // $NON-NLS-1$
-                if (hasAnnotationOnMethod(annotations,s, contextClassLoader)) {
-                    return super.add(s);
+        public boolean accept(String className) {
+            if ((className.indexOf('$') == -1) || inner) { // $NON-NLS-1$
+                if (hasAnnotationOnMethod(annotations,className, contextClassLoader)) {
+                    return true;
                 }
             }
             return false;
@@ -276,12 +250,31 @@ public final class ClassFinder {
             log.debug("contains: " + contains + " notContains: " + notContains);
         }
 
+        
+        ClassFilter filter = null;
+        if(annotations) {
+            @SuppressWarnings("unchecked") // Should only be called with classes that extend annotations
+            final Class<? extends Annotation>[] annoclassNames = (Class<? extends Annotation>[]) classNames;
+            filter = new AnnoClassFilter(annoclassNames, innerClasses);
+        }
+        else {
+            filter = new ExtendsClassFilter(classNames, innerClasses, contains, notContains);
+        }
+        
+        return findClasses(searchPathsOrJars, filter);
+    }
+    
+    public static List<String> findClasses(String[] searchPathsOrJars, ClassFilter filter) throws IOException  {
+        if (log.isDebugEnabled()) {
+            log.debug("searchPathsOrJars : " + Arrays.toString(searchPathsOrJars));
+        }
+    
         // Find all jars in the search path
         String[] strPathsOrJars = addJarsInPath(searchPathsOrJars);
         for (int k = 0; k < strPathsOrJars.length; k++) {
             strPathsOrJars[k] = fixPathEntry(strPathsOrJars[k]);
         }
-
+    
         // Now eliminate any classpath entries that do not "match" the search
         List<String> listPaths = getClasspathMatches(strPathsOrJars);
         if (log.isDebugEnabled()) {
@@ -289,16 +282,13 @@ public final class ClassFinder {
                 log.debug("listPaths : " + path);
             }
         }
-
-        @SuppressWarnings("unchecked") // Should only be called with classes that extend annotations
-        final Class<? extends Annotation>[] annoclassNames = (Class<? extends Annotation>[]) classNames;
-        Set<String> listClasses =
-            annotations ?
-                new AnnoFilterTreeSet(annoclassNames, innerClasses)
-                :
-                new FilterTreeSet(classNames, innerClasses, contains, notContains);
+    
+        Set<String> listClasses = new TreeSet<>();
         // first get all the classes
-        findClassesInPaths(listPaths, listClasses);
+        for (String path : listPaths) {
+            findClassesInOnePath(path, listClasses, filter);
+        }
+        
         if (log.isDebugEnabled()) {
             log.debug("listClasses.size()="+listClasses.size());
             for (String clazz : listClasses) {
@@ -440,10 +430,11 @@ public final class ClassFinder {
         return strClassName;
     }
 
-    private static void findClassesInOnePath(String strPath, Set<String> listClasses) throws IOException {
+    
+    private static void findClassesInOnePath(String strPath, Set<String> listClasses, ClassFilter filter) throws IOException {
         File file = new File(strPath);
         if (file.isDirectory()) {
-            findClassesInPathsDir(strPath, file, listClasses);
+            findClassesInPathsDir(strPath, file, listClasses, filter);
         } else if (file.exists()) {
             ZipFile zipFile = null;
             try {
@@ -452,7 +443,10 @@ public final class ClassFinder {
                 while (entries.hasMoreElements()) {
                     String strEntry = entries.nextElement().toString();
                     if (strEntry.endsWith(DOT_CLASS)) {
-                        listClasses.add(fixClassName(strEntry));
+                        String fixedClassName = fixClassName(strEntry);
+                        if(filter.accept(fixedClassName)) {
+                            listClasses.add(fixedClassName);
+                        }
                     }
                 }
             } catch (IOException e) {
@@ -466,29 +460,30 @@ public final class ClassFinder {
         }
     }
 
-    private static void findClassesInPaths(List<String> listPaths, Set<String> listClasses) throws IOException {
-        for (String path : listPaths) {
-            findClassesInOnePath(path, listClasses);
-        }
-    }
 
-    private static void findClassesInPathsDir(String strPathElement, File dir, Set<String> listClasses) throws IOException {
+    private static void findClassesInPathsDir(String strPathElement, File dir, Set<String> listClasses, ClassFilter filter) throws IOException {
         String[] list = dir.list();
-        if(list==null) {
+        if(list == null) {
             log.warn(dir.getAbsolutePath()+" is not a folder");
             return;
         }
+        
         for (String aList : list) {
             File file = new File(dir, aList);
             if (file.isDirectory()) {
                 // Recursive call
-                findClassesInPathsDir(strPathElement, file, listClasses);
-            } else if (aList.endsWith(DOT_CLASS) && file.exists() && (file.length() != 0)) {
+                findClassesInPathsDir(strPathElement, file, listClasses, filter);
+            }
+            else if (aList.endsWith(DOT_CLASS) && file.exists() && (file.length() != 0)) {
                 final String path = file.getPath();
-                listClasses.add(path.substring(strPathElement.length() + 1,
+                String className = path.substring(strPathElement.length() + 1,
                         path.lastIndexOf('.')) // $NON-NLS-1$
-                        .replace(File.separator.charAt(0), '.')); // $NON-NLS-1$
+                        .replace(File.separator.charAt(0), '.');// $NON-NLS-1$
+                if(filter.accept(className)) {
+                    listClasses.add(className);
+                }
             }
         }
     }
+    
 }

Modified: jmeter/trunk/test/src/org/apache/jmeter/gui/util/TestMenuFactory.java
URL: http://svn.apache.org/viewvc/jmeter/trunk/test/src/org/apache/jmeter/gui/util/TestMenuFactory.java?rev=1727442&r1=1727441&r2=1727442&view=diff
==============================================================================
--- jmeter/trunk/test/src/org/apache/jmeter/gui/util/TestMenuFactory.java (original)
+++ jmeter/trunk/test/src/org/apache/jmeter/gui/util/TestMenuFactory.java Thu Jan 28 22:23:23 2016
@@ -18,10 +18,16 @@
 
 package org.apache.jmeter.gui.util;
 
+import java.awt.GraphicsEnvironment;
+
 import org.apache.jmeter.junit.JMeterTestCase;
+import org.apache.jorphan.logging.LoggingManager;
+import org.apache.log.Logger;
 
 public final class TestMenuFactory extends JMeterTestCase {
 
+    private static final Logger log = LoggingManager.getLoggerForClass();
+    
         public TestMenuFactory() {
             super();
         }
@@ -35,6 +41,11 @@ public final class TestMenuFactory exten
         }
 
         public void testMenu() throws Exception {
+            if(GraphicsEnvironment.isHeadless()) {
+                System.out.println("Skipping test:"+getClass().getName()+"#testCloneSampler"+", cannot run in Headless mode");
+                log.warn("Skipping test:"+getClass().getName()+"#testCloneSampler"+", cannot run in Headless mode");
+                return;
+            }
             check("menumap", MenuFactory.menuMap_size());
 
             check("assertions", MenuFactory.assertions_size());

Modified: jmeter/trunk/test/src/org/apache/jmeter/junit/JMeterTest.java
URL: http://svn.apache.org/viewvc/jmeter/trunk/test/src/org/apache/jmeter/junit/JMeterTest.java?rev=1727442&r1=1727441&r2=1727442&view=diff
==============================================================================
--- jmeter/trunk/test/src/org/apache/jmeter/junit/JMeterTest.java (original)
+++ jmeter/trunk/test/src/org/apache/jmeter/junit/JMeterTest.java Thu Jan 28 22:23:23 2016
@@ -19,6 +19,7 @@
 package org.apache.jmeter.junit;
 
 import java.awt.Component;
+import java.awt.GraphicsEnvironment;
 import java.awt.HeadlessException;
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
@@ -129,12 +130,18 @@ public class JMeterTest extends JMeterTe
      * Use a suite to allow the tests to be generated at run-time
      */
     public static Test suite() throws Exception {
+        TestSuite suite = new TestSuite("JMeterTest");
+        if(GraphicsEnvironment.isHeadless()) {
+            System.out.println("Skipping test:"+JMeterTest.class.getName()+", cannot run in Headless mode");
+            log.warn("Skipping test:"+JMeterTest.class.getName()+", cannot run in Headless mode");
+            return suite;
+        }
+
         // The Locale used to instantiate the GUI objects
         JMeterUtils.setLocale(TEST_LOCALE);
         Locale.setDefault(TEST_LOCALE);
         // Needs to be done before any GUI classes are instantiated
         
-        TestSuite suite = new TestSuite("JMeterTest");
         suite.addTest(new JMeterTest("readAliases"));
         suite.addTest(new JMeterTest("createTitleSet"));
         suite.addTest(new JMeterTest("createTagSet"));

Modified: jmeter/trunk/test/src/org/apache/jorphan/test/AllTests.java
URL: http://svn.apache.org/viewvc/jmeter/trunk/test/src/org/apache/jorphan/test/AllTests.java?rev=1727442&r1=1727441&r2=1727442&view=diff
==============================================================================
--- jmeter/trunk/test/src/org/apache/jorphan/test/AllTests.java (original)
+++ jmeter/trunk/test/src/org/apache/jorphan/test/AllTests.java Thu Jan 28 22:23:23 2016
@@ -24,24 +24,26 @@ import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.InputStream;
 import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
 import java.nio.charset.Charset;
 import java.util.List;
 import java.util.Locale;
 import java.util.Properties;
 
-import junit.framework.Test;
-import junit.framework.TestCase;
-import junit.framework.TestResult;
-import junit.framework.TestSuite;
-import junit.textui.TestRunner;
-
 import org.apache.commons.lang3.exception.ExceptionUtils;
 import org.apache.jmeter.util.JMeterUtils;
 import org.apache.jorphan.logging.LoggingManager;
+import org.apache.jorphan.reflect.ClassFilter;
 import org.apache.jorphan.reflect.ClassFinder;
 import org.apache.jorphan.util.JOrphanUtils;
 import org.apache.log.Logger;
 
+import junit.framework.Test;
+import junit.framework.TestCase;
+import junit.framework.TestResult;
+import junit.framework.TestSuite;
+import junit.textui.TestRunner;
+
 /**
  * Provides a quick and easy way to run all <a
  * href="http://junit.sourceforge.net">junit</a> unit tests in your java
@@ -181,78 +183,18 @@ public final class AllTests {
         }
         log.info(sb.toString());
 
-        // ++
-        // GUI tests throw the error
-        // testArgumentCreation(org.apache.jmeter.config.gui.ArgumentsPanel$Test)java.lang.NoClassDefFoundError
-        // at java.lang.Class.forName0(Native Method)
-        // at java.lang.Class.forName(Class.java:141)
-        // at
-        // java.awt.GraphicsEnvironment.getLocalGraphicsEnvironment(GraphicsEnvironment.java:62)
-        //
-        // Try to find out why this is ...
-
         System.out.println("+++++++++++");
         logprop("java.awt.headless", true);
         logprop("java.awt.graphicsenv", true);
-        //
-        // try {//
-        // Class c = Class.forName(n);
-        // System.out.println("Found class: "+n);
-        // // c.newInstance();
-        // // System.out.println("Instantiated: "+n);
-        // } catch (Exception e1) {
-        // System.out.println("Error finding class "+n+" "+e1);
-        // } catch (java.lang.InternalError e1){
-        // System.out.println("Error finding class "+n+" "+e1);
-        // }
-        //
+        
         System.out.println("------------");
-        // don't call isHeadless() here, as it has a side effect.
-        // --
         System.out.println("Creating test suite");
         TestSuite suite = suite(args[0]);
+       
         int countTestCases = suite.countTestCases();
         System.out.println("Starting test run, test count = "+countTestCases);
-//        for (int i=0;i<suite.testCount();i++){
-//           Test testAt = suite.testAt(i);
-//           int testCases = testAt.countTestCases();
-//           if (testAt instanceof junit.framework.TestCase){
-//                System.out.print(((junit.framework.TestCase) testAt).getName());
-//            }
-//            if (testAt instanceof TestSuite){
-//                TestSuite testSuite = ((TestSuite) testAt);
-//                String name = testSuite.getName();
-//                System.out.print(name);
-//                System.out.println(" "+testCases);
-//            }                
-//        }
-        
-        // Jeremy Arnold: This method used to attempt to write results to
-        // a file, but it had a bug and instead just wrote to System.out.
-        // Since nobody has complained about this behavior, I'm changing
-        // the code to not attempt to write to a file, so it will continue
-        // behaving as it did before. It would be simple to make it write
-        // to a file instead if that is the desired behavior.
         TestResult result = TestRunner.run(suite);
-        // ++
-        // Recheck settings:
-        //System.out.println("+++++++++++");
-        // System.out.println(e+"="+System.getProperty(e));
-        // System.out.println(g+"="+System.getProperty(g));
-        // System.out.println("Headless?
-        // "+java.awt.GraphicsEnvironment.isHeadless());
-        // try {
-        // Class c = Class.forName(n);
-        // System.out.println("Found class: "+n);
-        // c.newInstance();
-        // System.out.println("Instantiated: "+n);
-        // } catch (Exception e1) {
-        // System.out.println("Error with class "+n+" "+e1);
-        // } catch (java.lang.InternalError e1){
-        // System.out.println("Error with class "+n+" "+e1);
-        // }
-        //System.out.println("------------");
-        // --
+        
         System.exit(result.wasSuccessful() ? 0 : 1); // this is needed because the test may start the AWT EventQueue thread which is not a daemon.
     }
 
@@ -335,8 +277,7 @@ public final class AllTests {
         int suites=0;
         try {
             log.info("ClassFinder(TestCase)");
-            List<String> classList = ClassFinder.findClassesThatExtend(JOrphanUtils.split(searchPaths, ","),
-                    new Class[] { TestCase.class }, true);
+            List<String> classList = findJMeterJUnitTests(searchPaths);
             int sz=classList.size();
             log.info("ClassFinder(TestCase) found: "+sz+ " TestCase classes");
             System.out.println("ClassFinder found: "+sz+ " TestCase classes");
@@ -382,4 +323,42 @@ public final class AllTests {
         System.out.println("Created: "+tests+" tests including "+suites+" suites");
         return suite;
     }
+
+    private static List<String> findJMeterJUnitTests(String searchPaths)  throws IOException {
+        List<String> classList = ClassFinder.findClasses(JOrphanUtils.split(searchPaths, ","), new JunitTestFilter());
+       
+        return classList;
+    }
+    
+    /**
+     * find the junit tests in the test search path
+     */
+    private static class JunitTestFilter implements ClassFilter {
+        
+        private final transient ClassLoader contextClassLoader = Thread.currentThread().getContextClassLoader();
+
+        @Override
+        public boolean accept(String className) {
+            
+            boolean isJunitTest = false;
+            try {
+                Class<?> c = Class.forName(className, false, contextClassLoader);
+
+                if (!c.isInterface() && !Modifier.isAbstract(c.getModifiers())) {
+                    if (TestCase.class.isAssignableFrom(c)) {
+                        isJunitTest =  true;
+                    }
+                }
+            } catch (UnsupportedClassVersionError ignored) {
+                log.debug(ignored.getLocalizedMessage());
+            } catch (NoClassDefFoundError ignored) {
+                log.debug(ignored.getLocalizedMessage());
+            } catch (ClassNotFoundException ignored) {
+                log.debug(ignored.getLocalizedMessage());
+            }
+            
+            return isJunitTest;
+        }
+        
+    }
 }

Modified: jmeter/trunk/xdocs/changes.xml
URL: http://svn.apache.org/viewvc/jmeter/trunk/xdocs/changes.xml?rev=1727442&r1=1727441&r2=1727442&view=diff
==============================================================================
--- jmeter/trunk/xdocs/changes.xml (original)
+++ jmeter/trunk/xdocs/changes.xml Thu Jan 28 22:23:23 2016
@@ -211,6 +211,7 @@ Summary
 <li><bug>57110</bug>Fixed spelling+grammar, formatting, removed commented out code etc. Contributed by Graham Russell (jmeter at ham1.co.uk)</li>
 <li>Correct instructions on running jmeter in help.txt. Contributed by Pascal Schumacher (pascalschumacher at gmx.net)</li>
 <li><bug>58704</bug>Non regression testing : Ant task batchtest fails if tests and run in a non en_EN locale and use a JMX file that uses a Csv DataSet</li>
+<li><bug>58897</bug>Improve JUnit Test code. Contributed by Benoit Wiart (benoit dot wiart at gmail.com)</li>
 </ul>
  
  <!-- =================== Bug fixes =================== -->