Merge pull request #142 from AndreiZaitcev/failed_tests_counter_fix

Fixes a bug when a number of failed tests will be negative

fork-common/src/main/java/com/shazam/fork/model/TestCaseEvent.java

@@ -4,51 +4,52 @@
 import com.google.common.base.Objects;
 
 import javax.annotation.Nonnull;
+import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
-import static java.util.Collections.emptyList;
-import static java.util.Collections.emptyMap;
+import static com.shazam.fork.model.TestCaseEvent.Builder.testCaseEvent;
+import static java.util.Collections.unmodifiableList;
+import static java.util.Collections.unmodifiableMap;
 import static org.apache.commons.lang3.builder.ToStringBuilder.reflectionToString;
 import static org.apache.commons.lang3.builder.ToStringStyle.SIMPLE_STYLE;
 
 public class TestCaseEvent {
-
     private final String testMethod;
     private final String testClass;
     private final boolean isIgnored;
     private final List<String> permissionsToRevoke;
     private final Map<String, String> properties;
 
-    private TestCaseEvent(String testMethod, String testClass, boolean isIgnored, List<String> permissionsToRevoke, Map<String, String> properties) {
-        this.testMethod = testMethod;
-        this.testClass = testClass;
-        this.isIgnored = isIgnored;
-        this.permissionsToRevoke = permissionsToRevoke;
-        this.properties = properties;
-    }
-
-    public static TestCaseEvent newTestCase(String testMethod, String testClass, boolean isIgnored, List<String> permissionsToRevoke, Map<String, String> properties) {
-        return new TestCaseEvent(testMethod, testClass, isIgnored, permissionsToRevoke, properties);
+    private TestCaseEvent(Builder builder) {
+        this.testClass = builder.testClass;
+        this.testMethod = builder.testMethod;
+        this.isIgnored = builder.isIgnored;
+        this.permissionsToRevoke = builder.permissionsToRevoke;
+        this.properties = builder.properties;
     }
 
-    public static TestCaseEvent newTestCase(@Nonnull TestIdentifier testIdentifier) {
-        return newTestCase(testIdentifier, false);
-    }
-
-    public static TestCaseEvent newTestCase(@Nonnull TestIdentifier testIdentifier, boolean isIgnored) {
-        return new TestCaseEvent(testIdentifier.getTestName(), testIdentifier.getClassName(), isIgnored,
-                emptyList(), emptyMap());
+    @Nonnull
+    public static TestCaseEvent from(@Nonnull TestIdentifier testIdentifier) {
+        return testCaseEvent()
+                .withTestClass(testIdentifier.getClassName())
+                .withTestMethod(testIdentifier.getTestName())
+                .withIsIgnored(false)
+                .build();
     }
 
+    @Nonnull
     public String getTestFullName() {
         return testClass + "#" + testMethod;
     }
 
+    @Nonnull
     public String getTestMethod() {
         return testMethod;
     }
 
+    @Nonnull
     public String getTestClass() {
         return testClass;
     }
@@ -57,12 +58,14 @@ public boolean isIgnored() {
         return isIgnored;
     }
 
+    @Nonnull
     public List<String> getPermissionsToRevoke() {
-        return permissionsToRevoke;
+        return unmodifiableList(permissionsToRevoke);
     }
 
+    @Nonnull
     public Map<String, String> getProperties() {
-        return properties;
+        return unmodifiableMap(properties);
     }
 
     @Override
@@ -87,4 +90,61 @@ public boolean equals(Object obj) {
     public String toString() {
         return reflectionToString(this, SIMPLE_STYLE);
     }
+
+    public static class Builder {
+        private String testClass;
+        private String testMethod;
+        private boolean isIgnored;
+        private List<String> permissionsToRevoke = new ArrayList<>();
+        private Map<String, String> properties = new HashMap<>();
+
+        @Nonnull
+        public static Builder testCaseEvent(@Nonnull TestIdentifier testIdentifier) {
+            return testCaseEvent()
+                    .withTestClass(testIdentifier.getClassName())
+                    .withTestMethod(testIdentifier.getTestName());
+        }
+
+        @Nonnull
+        public static Builder testCaseEvent() {
+            return new Builder();
+        }
+
+        @Nonnull
+        public Builder withTestClass(@Nonnull String testClass) {
+            this.testClass = testClass;
+            return this;
+        }
+
+        @Nonnull
+        public Builder withTestMethod(@Nonnull String testMethod) {
+            this.testMethod = testMethod;
+            return this;
+        }
+
+        @Nonnull
+        public Builder withIsIgnored(boolean isIgnored) {
+            this.isIgnored = isIgnored;
+            return this;
+        }
+
+        @Nonnull
+        public Builder withPermissionsToRevoke(@Nonnull List<String> permissionsToRevoke) {
+            this.permissionsToRevoke.clear();
+            this.permissionsToRevoke.addAll(permissionsToRevoke);
+            return this;
+        }
+
+        @Nonnull
+        public Builder withProperties(@Nonnull Map<String, String> properties) {
+            this.properties.clear();
+            this.properties.putAll(properties);
+            return this;
+        }
+
+        @Nonnull
+        public TestCaseEvent build() {
+            return new TestCaseEvent(this);
+        }
+    }
 }

fork-common/src/main/java/com/shazam/fork/suite/TestSuiteLoader.java

@@ -12,17 +12,25 @@
 
 import com.shazam.fork.io.DexFileExtractor;
 import com.shazam.fork.model.TestCaseEvent;
-import org.jf.dexlib.*;
+import org.jf.dexlib.AnnotationDirectoryItem;
+import org.jf.dexlib.AnnotationItem;
+import org.jf.dexlib.AnnotationSetItem;
+import org.jf.dexlib.ClassDefItem;
 import org.jf.dexlib.EncodedValue.AnnotationEncodedSubValue;
 import org.jf.dexlib.EncodedValue.ArrayEncodedValue;
 import org.jf.dexlib.EncodedValue.EncodedValue;
 import org.jf.dexlib.EncodedValue.StringEncodedValue;
+import org.jf.dexlib.StringIdItem;
 
 import javax.annotation.Nonnull;
 import java.io.File;
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
 
-import static com.shazam.fork.model.TestCaseEvent.newTestCase;
+import static com.shazam.fork.model.TestCaseEvent.Builder.testCaseEvent;
 import static java.lang.Math.min;
 import static java.util.Arrays.stream;
 import static java.util.Collections.emptyList;
@@ -83,10 +91,16 @@ private TestCaseEvent convertToTestCaseEvent(ClassDefItem classDefItem,
         String testMethod = method.method.getMethodName().getStringValue();
         AnnotationItem[] annotations = method.annotationSet.getAnnotations();
         String testClass = getClassName(classDefItem);
-        boolean ignored = isClassIgnored(annotationDirectoryItem) || isMethodIgnored(annotations);
+        boolean isIgnored = isClassIgnored(annotationDirectoryItem) || isMethodIgnored(annotations);
         List<String> permissionsToRevoke = getPermissionsToRevoke(annotations);
         Map<String, String> properties = getTestProperties(annotations);
-        return newTestCase(testMethod, testClass, ignored, permissionsToRevoke, properties);
+        return testCaseEvent()
+                .withTestClass(testClass)
+                .withTestMethod(testMethod)
+                .withIsIgnored(isIgnored)
+                .withPermissionsToRevoke(permissionsToRevoke)
+                .withProperties(properties)
+                .build();
     }
 
     private String getClassName(ClassDefItem classDefItem) {

fork-common/src/test/java/com/shazam/fork/suite/TestSuiteLoaderTest.java

@@ -26,44 +26,45 @@
 
 import static com.shazam.fork.io.FakeDexFileExtractor.fakeDexFileExtractor;
 import static com.shazam.fork.io.Files.convertFileToDexFile;
-import static com.shazam.fork.model.TestCaseEvent.newTestCase;
+import static com.shazam.fork.model.TestCaseEvent.Builder.testCaseEvent;
 import static com.shazam.fork.suite.FakeTestClassMatcher.fakeTestClassMatcher;
 import static com.shazam.shazamcrest.MatcherAssert.assertThat;
 import static com.shazam.shazamcrest.matcher.Matchers.sameBeanAs;
 import static java.util.Arrays.asList;
-import static java.util.Collections.*;
+import static java.util.Collections.emptyMap;
+import static java.util.Collections.singletonMap;
 import static org.hamcrest.Matchers.hasItems;
 
 /**
  * This test is based on the <code>tests.dex</code> file, which contains test classes with the following code:
  * <blockquote><pre>
- *{@literal}@Ignore
- *public class IgnoredClassTest {
+ * {@literal}@Ignore
+ * public class IgnoredClassTest {
  *    {@literal}@Test
  *    public void methodOfAnIgnoredTestClass() {
  *    }
- *}
- *
- *public class ClassWithNoIgnoredMethodsTest {
+ * }
+ * <p>
+ * public class ClassWithNoIgnoredMethodsTest {
  *    {@literal}@Test
  *    public void firstTestMethod() {
  *    }
- *
+ * <p>
  *    {@literal}@Test
  *    public void secondTestMethod() {
  *    }
- *}
- *
- *public class ClassWithSomeIgnoredMethodsTest {
+ * }
+ * <p>
+ * public class ClassWithSomeIgnoredMethodsTest {
  *    {@literal}@Test
  *    public void nonIgnoredTestMethod() {
  *    }
- *
+ * <p>
  *    {@literal}@Test
  *    {@literal}@Ignore
  *    public void ignoredTestMethod() {
  *    }
- *}
+ * }
  * </pre></blockquote>
  */
 public class TestSuiteLoaderTest {
@@ -81,7 +82,7 @@ private DexFile testDexFile() {
     }
 
     @Before
-    public void setUp() throws Exception {
+    public void setUp() {
         testSuiteLoader = new TestSuiteLoader(ANY_INSTRUMENTATION_APK_FILE, fakeDexFileExtractor, fakeTestClassMatcher);
     }
 
@@ -125,16 +126,35 @@ public void populatesTestProperties() throws Exception {
 
     @Nonnull
     private Matcher<TestCaseEvent> sameTestEventAs(String testMethod, String testClass, Map<String, String> properties) {
-        return sameBeanAs(newTestCase(testMethod, testClass, false, emptyList(), properties));
+        return sameBeanAs(
+                testCaseEvent()
+                        .withTestClass(testClass)
+                        .withTestMethod(testMethod)
+                        .withProperties(properties)
+                        .build()
+        );
     }
 
     @Nonnull
     private Matcher<TestCaseEvent> sameTestEventAs(String testMethod, String testClass, boolean isIgnored) {
-        return sameBeanAs(newTestCase(testMethod, testClass, isIgnored, emptyList(), emptyMap()));
+        return sameBeanAs(
+                testCaseEvent()
+                        .withTestClass(testClass)
+                        .withTestMethod(testMethod)
+                        .withIsIgnored(isIgnored)
+                        .build()
+        );
     }
 
     @Nonnull
     private Matcher<TestCaseEvent> sameTestEventAs(String testMethod, String testClass, boolean isIgnored, List<String> permissions) {
-        return sameBeanAs(newTestCase(testMethod, testClass, isIgnored, permissions, emptyMap()));
+        return sameBeanAs(
+                testCaseEvent()
+                        .withTestClass(testClass)
+                        .withTestMethod(testMethod)
+                        .withIsIgnored(isIgnored)
+                        .withPermissionsToRevoke(permissions)
+                        .build()
+        );
     }
 }

fork-runner/src/main/java/com/shazam/fork/device/DeviceTestFilesCleaner.java

@@ -1,7 +1,9 @@
 package com.shazam.fork.device;
 
-import com.android.ddmlib.testrunner.TestIdentifier;
+import com.shazam.fork.model.TestCaseEvent;
+
+import javax.annotation.Nonnull;
 
 public interface DeviceTestFilesCleaner {
-    boolean deleteTraceFiles(TestIdentifier testIdentifier);
+    boolean deleteTraceFiles(@Nonnull TestCaseEvent testCase);
 }

fork-runner/src/main/java/com/shazam/fork/device/DeviceTestFilesCleanerImpl.java → fork-runner/src/main/java/com/shazam/fork/device/FileManagerBasedDeviceTestFilesCleaner.java

@@ -1,30 +1,31 @@
 package com.shazam.fork.device;
 
-import com.android.ddmlib.testrunner.TestIdentifier;
 import com.shazam.fork.model.Device;
 import com.shazam.fork.model.Pool;
+import com.shazam.fork.model.TestCaseEvent;
 import com.shazam.fork.system.io.FileManager;
 import com.shazam.fork.system.io.FileType;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import javax.annotation.Nonnull;
 import java.io.File;
 
-public class DeviceTestFilesCleanerImpl implements DeviceTestFilesCleaner {
-    private static final Logger logger = LoggerFactory.getLogger(DeviceTestFilesCleanerImpl.class);
+public class FileManagerBasedDeviceTestFilesCleaner implements DeviceTestFilesCleaner {
+    private static final Logger logger = LoggerFactory.getLogger(FileManagerBasedDeviceTestFilesCleaner.class);
     private final FileManager fileManager;
     private final Pool pool;
     private final Device device;
 
-    public DeviceTestFilesCleanerImpl(FileManager fileManager, Pool pool, Device device) {
+    public FileManagerBasedDeviceTestFilesCleaner(FileManager fileManager, Pool pool, Device device) {
         this.fileManager = fileManager;
         this.pool = pool;
         this.device = device;
     }
 
     @Override
-    public boolean deleteTraceFiles(TestIdentifier testIdentifier) {
-        File file = fileManager.getFile(FileType.TEST, pool.getName(), device.getSafeSerial(), testIdentifier);
+    public boolean deleteTraceFiles(@Nonnull TestCaseEvent testCase) {
+        File file = fileManager.getFile(FileType.TEST, pool, device, testCase);
         boolean isDeleted = file.delete();
         if (!isDeleted) {
             logger.warn("Failed to delete a file %s", file.getAbsolutePath());