Sonar fixes (#43)

sonar fixes
Substeps · Feb 27, 2017 · 07e7518 · 07e7518
1 parent 5ea2995
commit 07e7518
Show file tree

Hide file tree

Showing 34 changed files with 153 additions and 446 deletions.
diff --git a/README.md b/README.md
@@ -27,6 +27,7 @@ Requirements
 * Improved error message when no tests are run
 * escaped < and > in the glossary data
 * Corrected the path to the screenshot images to be relative so that they work when served off a server and locally.
+* various sonar fixes
 
 1.0.5
 -----

diff --git a/api/src/main/java/com/technophobia/substeps/execution/node/ExecutionNode.java b/api/src/main/java/com/technophobia/substeps/execution/node/ExecutionNode.java
@@ -201,18 +201,22 @@ public String toDebugString() {
         return debugString;
     }
 
+    @Override
     public String getSourceLine() {
         return sourceLine;
     }
 
+    @Override
     public void setSourceLine(String sourceLine) {
         this.sourceLine = sourceLine;
     }
 
+    @Override
     public void setParameterNames(List<String> parameterNames) {
         this.parameterNames = parameterNames;
     }
 
+    @Override
     public List<String> getParameterNames() {
         return parameterNames;
     }

diff --git a/api/src/main/java/com/technophobia/substeps/execution/node/StepImplementationNode.java b/api/src/main/java/com/technophobia/substeps/execution/node/StepImplementationNode.java
@@ -29,6 +29,7 @@ public class StepImplementationNode extends ExecutionNode implements StepNode {
 
     private static final long serialVersionUID = 1L;
 
+    // these attributes are not required when serialized, just the string versions of them below.
     private final transient Class<?> targetClass;
     private final transient Method targetMethod;
 

diff --git a/api/src/main/java/com/technophobia/substeps/model/Configuration.java b/api/src/main/java/com/technophobia/substeps/model/Configuration.java
@@ -32,9 +32,9 @@ public enum Configuration {
 
     INSTANCE;
 
-    private final Logger logger;
+    private transient final Logger logger;
 
-    private final Config config;
+    private transient final Config config;
 
     Configuration() {
         logger = LoggerFactory.getLogger(Configuration.class);

diff --git a/api/src/main/java/com/technophobia/substeps/model/exception/SubstepsRuntimeException.java b/api/src/main/java/com/technophobia/substeps/model/exception/SubstepsRuntimeException.java
@@ -30,5 +30,5 @@ public SubstepsRuntimeException(final String msg, Throwable cause) {
         super(msg, cause);
     }
 
-
+    public SubstepsRuntimeException(final Throwable cause) { super(cause);}
 }
diff --git a/api/src/main/java/com/technophobia/substeps/runner/ExecutionContext.java b/api/src/main/java/com/technophobia/substeps/runner/ExecutionContext.java
@@ -107,7 +107,6 @@ public static Map<String, Object> flatten(){
                 Sets.SetView intersection = Sets.intersection(newMaster.keySet(), scopedMap.keySet());
 
                 if (!intersection.isEmpty()) {
-                    StringBuilder buf = new StringBuilder();
                     intersection.stream().forEach(s -> log.warn("existing key value " + s + " is being overwritten flattening the ExecutionContext"));
                 }
 

diff --git a/api/src/main/java/com/technophobia/substeps/runner/SubstepExecutionFailure.java b/api/src/main/java/com/technophobia/substeps/runner/SubstepExecutionFailure.java
@@ -44,7 +44,6 @@ public Long apply(final SubstepExecutionFailure failure) {
 
     private static final long serialVersionUID = 4981517213059529046L;
 
-    private final transient Throwable cause;
     private IExecutionNode executionNode;
     private boolean setupOrTearDown = false;
     private boolean nonCritical = false;
@@ -54,7 +53,6 @@ public Long apply(final SubstepExecutionFailure failure) {
     private final ThrowableInfo throwableInfo;
 
     private SubstepExecutionFailure(Throwable cause, IExecutionNode executionNode, boolean setupOrTearDown, boolean nonCritical, byte[] screenshot) {
-        this.cause = cause;
         this.executionNode = executionNode;
         this.setupOrTearDown = setupOrTearDown;
         this.nonCritical = nonCritical;
@@ -84,21 +82,9 @@ public static SubstepExecutionFailure nonCriticalFailure(final Throwable cause,
         return sef;
     }
 
-    /*
-     * @param cause      the cause of the failure
-     * @param node       the node that failed execution
-     * @param screenshot the bytes representing the screenshot taken when the node execution failed
-     */
-//    public SubstepExecutionFailure(final Throwable cause, final IExecutionNode node, final byte[] screenshot) {
-//        this(cause, node);
-//        this.setScreenshot(screenshot);
-//    }
-
 
     public SubstepExecutionFailure(final Throwable cause) {
 
-        this.cause = cause;
-
         this.throwableInfo = new ThrowableInfo(cause);
     }
 
@@ -108,7 +94,6 @@ public SubstepExecutionFailure(final Throwable cause) {
      * @param node  the node that failed execution
      */
     public SubstepExecutionFailure(final Throwable cause, final IExecutionNode node) {
-        this.cause = cause;
         this.executionNode = node;
         this.executionNode.getResult().setFailure(this);
         this.throwableInfo = new ThrowableInfo(cause);
@@ -142,12 +127,8 @@ public SubstepExecutionFailure(final Throwable cause, final IExecutionNode node,
 
     public static void setResult(final Throwable cause, final IExecutionNode node, final ExecutionResult result) {
 
+        SubstepExecutionFailure.criticalFailure(cause, node, null);
 
-        final SubstepExecutionFailure sef = SubstepExecutionFailure.criticalFailure(cause, node, null);
-
-
-//        sef.executionNode = node;
-//        sef.executionNode.getResult().setFailure(sef);
         node.getResult().setResult(result);
     }
 
@@ -188,7 +169,7 @@ public void setSetupOrTearDown(final boolean setupOrTearDown) {
      * @return the cause
      */
     public Throwable getCause() {
-        return this.cause;
+        return this.throwableInfo.getThrowable();
     }
 
 

diff --git a/api/src/main/java/org/substeps/runner/CoreSubstepsPropertiesConfiguration.java b/api/src/main/java/org/substeps/runner/CoreSubstepsPropertiesConfiguration.java
@@ -28,7 +28,7 @@ public enum CoreSubstepsPropertiesConfiguration implements CoreSubstepsConfigura
 
     INSTANCE ; // uninstantiable
 
-    private final Logger LOG = LoggerFactory.getLogger(CoreSubstepsPropertiesConfiguration.class);
+    private transient final Logger LOG = LoggerFactory.getLogger(CoreSubstepsPropertiesConfiguration.class);
 
     private final int stepDepthForDescription;
 

diff --git a/core/pom.xml b/core/pom.xml
@@ -255,6 +255,14 @@
                 </execution></executions>
             </plugin>
 
+            <plugin>
+                <groupId>org.scoverage</groupId>
+                <artifactId>scoverage-maven-plugin</artifactId>
+                <configuration>
+                    <highlighting>true</highlighting>
+                </configuration>
+            </plugin>
+
         </plugins>
     </build>
 
@@ -280,7 +288,24 @@
                     <outputDirectory>${basedir}/target/newsite</outputDirectory>
                 </configuration>
             </plugin>
+
+            <plugin>
+                <groupId>org.scoverage</groupId>
+                <artifactId>scoverage-maven-plugin</artifactId>
+                <version>${scoverage.plugin.version}</version>
+                <reportSets>
+                    <reportSet>
+                        <reports>
+                            <report>report</report>
+                            <!-- or <report>integration-report</report> -->
+                            <!-- or <report>report-only</report> -->
+                        </reports>
+                    </reportSet>
+                </reportSets>
+            </plugin>
+
         </plugins>
+
     </reporting>
 
     <profiles>

diff --git a/core/src/main/java/com/technophobia/substeps/execution/DryRunImplementationCache.java b/core/src/main/java/com/technophobia/substeps/execution/DryRunImplementationCache.java
@@ -18,14 +18,12 @@
  */
 package com.technophobia.substeps.execution;
 
-import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 
 public class DryRunImplementationCache extends ImplementationCache {
 
     @Override
-    public void executeMethod(final Class<?> targetClass, final Method targetMethod, final Object[] methodArgs)
-            throws IllegalArgumentException, IllegalAccessException, InvocationTargetException {
+    public void executeMethod(final Class<?> targetClass, final Method targetMethod, final Object[] methodArgs){
 
         addImplementationClasses(targetClass);
 

diff --git a/core/src/main/java/com/technophobia/substeps/execution/ImplementationCache.java b/core/src/main/java/com/technophobia/substeps/execution/ImplementationCache.java
@@ -22,6 +22,7 @@
 import com.google.common.base.Predicate;
 import com.google.common.collect.Collections2;
 import com.google.common.collect.Maps;
+import com.technophobia.substeps.model.exception.SubstepsRuntimeException;
 
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
@@ -111,17 +112,20 @@ private Object instantiate(final Class<?> implementationClass) {
      * .util.List)
      */
     @Override
-    public void executeMethods(final List<Method> methods) throws Exception {
+    public void executeMethods(final List<Method> methods) {
 
         for (final Method method : methods) {
 
             // TODO - curious about the findSuitableInstancesOf ? won't
             // method.getDeclaringClass be ok??
 
             for (final Object object : findSuitableInstancesOf(method.getDeclaringClass())) {
-
-                method.invoke(object);
-
+                try {
+                    method.invoke(object);
+                }
+                catch (Exception e){
+                    throw new SubstepsRuntimeException(e);
+                }
             }
         }
     }
@@ -147,15 +151,19 @@ public boolean apply(final Class<?> instanceClass) {
      * .lang.Class, java.lang.reflect.Method, java.lang.Object[])
      */
     @Override
-    public void executeMethod(final Class<?> targetClass, final Method targetMethod, final Object[] methodArgs)
-            throws IllegalArgumentException, IllegalAccessException, InvocationTargetException {
+    public void executeMethod(final Class<?> targetClass, final Method targetMethod, final Object[] methodArgs){
 
         addImplementationClasses(targetClass);
 
-        if (methodArgs != null) {
-            targetMethod.invoke(instanceMap.get(targetClass), methodArgs);
-        } else {
-            targetMethod.invoke(instanceMap.get(targetClass));
+        try {
+            if (methodArgs != null) {
+                targetMethod.invoke(instanceMap.get(targetClass), methodArgs);
+            } else {
+                targetMethod.invoke(instanceMap.get(targetClass));
+            }
+        }
+        catch (Exception e){
+            throw new SubstepsRuntimeException(e);
         }
     }
 

diff --git a/core/src/main/java/com/technophobia/substeps/execution/MethodExecutor.java b/core/src/main/java/com/technophobia/substeps/execution/MethodExecutor.java
@@ -24,12 +24,11 @@
 
 public interface MethodExecutor {
 
-    void executeMethods(List<Method> setupAndTearDownMethods) throws Exception;
+    void executeMethods(List<Method> setupAndTearDownMethods) ;
 
     void addImplementationClasses(final Class<?>... implementationClasses);
 
     <T> T getImplementation(Class<T> implementationClass);
 
-    void executeMethod(final Class<?> targetClass, final Method targetMethod, final Object[] methodArgs)
-            throws IllegalArgumentException, IllegalAccessException, InvocationTargetException;
+    void executeMethod(final Class<?> targetClass, final Method targetMethod, final Object[] methodArgs);
 }
diff --git a/core/src/main/java/com/technophobia/substeps/execution/node/RootNodeExecutionContext.java b/core/src/main/java/com/technophobia/substeps/execution/node/RootNodeExecutionContext.java
@@ -77,8 +77,6 @@ public void addFailure(final SubstepExecutionFailure failure) {
 
         // set the criticality of this failure
         if (!failure.isSetupOrTearDown() && isNodeFailureNonCritical(failure.getExeccutionNode())) {
-//        if (!failure.isSetupOrTearDown() && this.nonFatalTagmanager != null
-//                && nonFatalTagmanager.isApplicable(failure.getExeccutionNode())) {
 
             failure.setNonCritical(true);
         }
@@ -90,7 +88,6 @@ public void addFailure(final SubstepExecutionFailure failure) {
      */
     private void logFailure(final SubstepExecutionFailure failure) {
 
-//        final Throwable failureCause = failure.getCause();
         final Throwable here = new Throwable();
 
         final StackTraceElement[] failureTrace = failure.getThrowableInfo().getStackTrace();

diff --git a/core/src/main/java/com/technophobia/substeps/model/Arguments.java b/core/src/main/java/com/technophobia/substeps/model/Arguments.java
@@ -149,7 +149,7 @@ public static String[] getArgs(final String patternString, final String sourceSt
 
                 if (arg != null) {
                     if (argsList == null) {
-                        argsList = new ArrayList<String>();
+                        argsList = new ArrayList<>();
                     }
                     argsList.add(arg);
                 }
@@ -200,23 +200,11 @@ public static List<Object> getArgs(final String patternString, final String sour
 
                 if (arg != null) {
                     if (argsList == null) {
-                        argsList = new ArrayList<Object>();
+                        argsList = new ArrayList<>();
                     }
                     String substituted = substituteValues(arg);
 
-//                    if (substituted.equals(arg)){
-//                        // no change, lets try against the context
-//                        Object result = evaluateExpression(arg);
-//                        if (result != null){
-//                            argsList.add(result);
-//                        }
-//                        else {
-//                            argsList.add(getObjectArg(substituted, parameterTypes[argIdx], converterTypes[argIdx]));
-//                        }
-//                    }
-//                    else {
                         argsList.add(getObjectArg(substituted, parameterTypes[argIdx], converterTypes[argIdx]));
-//                    }
                 }
                 argIdx++;
             }