You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by kl...@apache.org on 2015/12/22 20:26:31 UTC

[29/35] incubator-geode git commit: GEODE-660: Fixing inconsistencies and reliance on randomness in TableBuilderJUnitTest

GEODE-660: Fixing inconsistencies and reliance on randomness in TableBuilderJUnitTest


Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/c19c3cab
Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/c19c3cab
Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/c19c3cab

Branch: refs/heads/feature/GEODE-217
Commit: c19c3cab01e2c8975b35a563adc84e07bd42af4e
Parents: 5787a48
Author: Jens Deppe <jd...@pivotal.io>
Authored: Wed Dec 16 16:19:53 2015 -0800
Committer: Jens Deppe <jd...@pivotal.io>
Committed: Mon Dec 21 10:05:11 2015 -0800

----------------------------------------------------------------------
 gemfire-core/build.gradle                       |   8 +
 .../internal/cli/result/TableBuilder.java       |   9 +-
 .../internal/cli/result/TableBuilderHelper.java |  18 +-
 .../internal/cli/TableBuilderJUnitTest.java     | 307 +++++++++++++------
 gradle/dependency-versions.properties           |   2 +
 5 files changed, 246 insertions(+), 98 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/c19c3cab/gemfire-core/build.gradle
----------------------------------------------------------------------
diff --git a/gemfire-core/build.gradle b/gemfire-core/build.gradle
index 28fb3ba..3e3c19b 100755
--- a/gemfire-core/build.gradle
+++ b/gemfire-core/build.gradle
@@ -99,6 +99,14 @@ dependencies {
   testRuntime 'commons-io:commons-io:' + project.'commons-io.version'
   testCompile 'net.spy:spymemcached:' + project.'spymemcached.version'
   testCompile 'redis.clients:jedis:' + project.'jedis.version'
+
+  testCompile 'org.powermock:powermock-core:' + project.'powermock.version'
+  testCompile 'org.powermock:powermock-module-junit4:' + project.'powermock.version'
+  testCompile 'org.powermock:powermock-module-junit4-common:' + project.'powermock.version'
+  testCompile 'org.powermock:powermock-api-support:' + project.'powermock.version'
+  testCompile 'org.powermock:powermock-api-mockito:' + project.'powermock.version'
+  testRuntime 'org.powermock:powermock-reflect:' + project.'powermock.version'
+  testRuntime 'org.javassist:javassist:' + project.'javassist.version'
 }
 
 def generatedResources = "$buildDir/generated-resources/main"

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/c19c3cab/gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/cli/result/TableBuilder.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/cli/result/TableBuilder.java b/gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/cli/result/TableBuilder.java
index d2363de..dfa4b3e 100644
--- a/gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/cli/result/TableBuilder.java
+++ b/gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/cli/result/TableBuilder.java
@@ -104,7 +104,14 @@ public class TableBuilder {
       RowGroup rowGroup = newRowGroup();
       rowGroup.newBlankRow();
     }
-    
+
+    public RowGroup getLastRowGroup() {
+      if (rowGroups.size() == 0) {
+        return null;
+      }
+      return rowGroups.get(rowGroups.size() - 1);
+    }
+
     /**
      * Computes total Max Row Length across table - for all row groups.
      */

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/c19c3cab/gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/cli/result/TableBuilderHelper.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/cli/result/TableBuilderHelper.java b/gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/cli/result/TableBuilderHelper.java
index 32fa048..aa4029f 100644
--- a/gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/cli/result/TableBuilderHelper.java
+++ b/gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/cli/result/TableBuilderHelper.java
@@ -31,6 +31,8 @@ import com.gemstone.gemfire.management.internal.cli.shell.Gfsh;
 
 public class TableBuilderHelper {
 
+  private static final int SCREEN_WIDTH_MARGIN_BUFFER = 5;
+
   public static class Column implements Comparable<Column>{
     int length;
     int originalIndex;
@@ -85,21 +87,19 @@ public class TableBuilderHelper {
       int totalExtra = 0;
       for (Column s : stringList) {
         int newLength = totalLength + s.length;
-        if (newLength > screenWidth) {
+        // Ensure that the spaceLeft is never < 2 which would prevent displaying a trimmed value
+        // even when there is space available on the screen.
+        if (newLength + SCREEN_WIDTH_MARGIN_BUFFER > screenWidth) {
           s.markForTrim = true;
           totalExtra += s.length;
-          if (spaceLeft == 0)
+          if (spaceLeft == 0) {
             spaceLeft = screenWidth - totalLength;
+          }
         }
         totalLength = newLength;
       }
 
-      Collections.sort(stringList, new Comparator<Column>() {
-        @Override
-        public int compare(Column o1, Column o2) {
-          return o1.originalIndex - o2.originalIndex;
-        }
-      });
+      Collections.sort(stringList, (o1, o2) -> o1.originalIndex - o2.originalIndex);
 
       //calculate trimmed width for columns marked for
       //distribute the trimming as per percentage
@@ -129,7 +129,7 @@ public class TableBuilderHelper {
           throw new TooManyColumnsException(
               "Computed ColSize="
                   + colSize
-                  + " Set RESULT_VIEWER to external (uses less commands for enabling horizontal scroll) to see wider results");
+                  + " Set RESULT_VIEWER to external. This uses the 'less' command (with horizontal scrolling) to see wider results");
         totalLength += colSize;
         index++;
       }

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/c19c3cab/gemfire-core/src/test/java/com/gemstone/gemfire/management/internal/cli/TableBuilderJUnitTest.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/test/java/com/gemstone/gemfire/management/internal/cli/TableBuilderJUnitTest.java b/gemfire-core/src/test/java/com/gemstone/gemfire/management/internal/cli/TableBuilderJUnitTest.java
index e5f1d86..ad23427 100644
--- a/gemfire-core/src/test/java/com/gemstone/gemfire/management/internal/cli/TableBuilderJUnitTest.java
+++ b/gemfire-core/src/test/java/com/gemstone/gemfire/management/internal/cli/TableBuilderJUnitTest.java
@@ -21,163 +21,294 @@ import com.gemstone.gemfire.management.internal.cli.result.TableBuilder.Row;
 import com.gemstone.gemfire.management.internal.cli.result.TableBuilder.RowGroup;
 import com.gemstone.gemfire.management.internal.cli.result.TableBuilder.Table;
 import com.gemstone.gemfire.management.internal.cli.result.TableBuilderHelper;
-import com.gemstone.gemfire.management.internal.cli.shell.Gfsh;
 import com.gemstone.gemfire.test.junit.categories.IntegrationTest;
-import org.junit.Ignore;
+import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 import org.junit.rules.TestName;
+import org.junit.runner.RunWith;
+import org.powermock.core.classloader.annotations.PowerMockIgnore;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
 
-import java.io.IOException;
-import java.util.Properties;
-import java.util.Random;
+import java.util.Arrays;
+import java.util.List;
 
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
+import static org.junit.Assert.*;
+import static org.powermock.api.mockito.PowerMockito.spy;
+import static org.powermock.api.mockito.PowerMockito.when;
 
 /**
- * TODO: fails when running integrationTest from gradle command-line or in Eclipse on Windows 7
- * <p>
- * com.gemstone.gemfire.management.internal.cli.TableBuilderJUnitTest > testBasicScrapping FAILED
- * java.lang.AssertionError: Expected length < 100 is 101 at org.junit.Assert.fail(Assert.java:88) at
- * com.gemstone.gemfire.management.internal.cli.TableBuilderJUnitTest.doTableBuilderTestUnit(TableBuilderJUnitTest.java:115)
- * at com.gemstone.gemfire.management.internal.cli.TableBuilderJUnitTest.testBasicScrapping(TableBuilderJUnitTest.java:134)
- * <p>
- * com.gemstone.gemfire.management.internal.cli.TableBuilderJUnitTest > testManyColumns FAILED java.lang.AssertionError:
- * Expected length < 100 is 101 at org.junit.Assert.fail(Assert.java:88) at com.gemstone.gemfire.management.internal.cli.TableBuilderJUnitTest.doTableBuilderTestUnit(TableBuilderJUnitTest.java:115)
- * at com.gemstone.gemfire.management.internal.cli.TableBuilderJUnitTest.testManyColumns(TableBuilderJUnitTest.java:155)
+ * Testing TableBuilder and TableBuilderHelper using mocks for Gfsh
  *
  * @author tushark
+ * @author Jens Deppe
  */
 @Category(IntegrationTest.class)
+@RunWith(PowerMockRunner.class)
+@PowerMockIgnore("*.IntegrationTest")
+@PrepareForTest(TableBuilderHelper.class)
 public class TableBuilderJUnitTest {
 
   @Rule
   public TestName testName = new TestName();
 
-  private final Table createTable(int rows, int cols, int width, String separator) {
+  @Before
+  public void setUp() throws Exception {
+    // This sets up a partial mock for some static methods
+    spy(TableBuilderHelper.class);
+    when(TableBuilderHelper.class, "getScreenWidth").thenReturn(40);
+    when(TableBuilderHelper.class, "shouldTrimColumns").thenReturn(true);
+  }
+
+  private Table createTableStructure(int cols, String separator) {
+    String[] colNames = new String[cols];
+    for (int i = 0; i < cols; i++) {
+      colNames[i] = "Field";
+    }
+    return createTableStructure(cols, separator, colNames);
+  }
+
+  private Table createTableStructure(int cols, String separator, String... colNames) {
     Table resultTable = TableBuilder.newTable();
     resultTable.setTabularResult(true);
     resultTable.setColumnSeparator(separator);
 
     resultTable.newBlankRow();
-    resultTable.newRow().newLeftCol("Displaying all fields for member: ");
-    resultTable.newBlankRow();
     RowGroup rowGroup = resultTable.newRowGroup();
     Row row = rowGroup.newRow();
     for (int colIndex = 0; colIndex < cols; colIndex++) {
-      row.newCenterCol("Field" + colIndex);
+      row.newCenterCol(colNames[colIndex] + colIndex);
     }
 
     rowGroup.newRowSeparator('-', false);
 
-    int counter = rows;
-    for (int i = 0; i < counter; i++) {
-      row = rowGroup.newRow();
-      for (int k = 0; k < cols; k++) {
-        row.newLeftCol(getString(i, width / cols));
-      }
-    }
-    resultTable.newBlankRow();
-
     return resultTable;
   }
 
-  private Object getString(int i, int width) {
-    StringBuilder sb = new StringBuilder();
-    Random random = new Random();
-    int k = 0;
-    double d = random.nextDouble();
-    // .09 probability
-    if (d <= 0.9) {
-      k = random.nextInt(width);
-    } else {
-      k = width / 2 + random.nextInt(width);
-    }
-    random.nextInt(10);
-    for (int j = 0; j < k; j++) {
-      sb.append(i);
-      if (sb.length() > k) break;
-    }
-    return sb.toString();
-  }
-
-  private HeadlessGfsh createShell(Properties props) throws ClassNotFoundException, IOException {
-    String shellId = getClass().getSimpleName() + "_" + testName;
-    HeadlessGfsh shell = new HeadlessGfsh(shellId, 30, props);
-    return shell;
-  }
-
-  private void doTableBuilderTestUnit(int rows, int cols, String sep, boolean shouldTrim,
-      boolean expectTooManyColEx) throws ClassNotFoundException, IOException {
-    int width = Gfsh.getCurrentInstance().getTerminalWidth();
-    Table table = createTable(rows, cols, width, sep);
+  private List<String> validateTable(Table table, boolean shouldTrim) {
+    int screenWidth = TableBuilderHelper.getScreenWidth();
     String st = table.buildTable();
     System.out.println(st);
 
-    String[] array = st.split("\n");
+    List<String> lines = Arrays.asList(st.split(GfshParser.LINE_SEPARATOR));
 
     int line = 0;
-    for (String s : array) {
-      System.out.println("For line " + line++ + " length is " + s.length() + " isWider = " + (s.length() > width));
+    for (String s : lines) {
+      System.out.println("For line " + line++ + " length is " + s.length() + " isWider = " + (s.length() > screenWidth));
 
       if (shouldTrim) {
-        if (s.length() > width) {
-          fail("Expected length < " + width + " is " + s.length());
+        if (s.length() > screenWidth) {
+          fail("Expected length > screenWidth: " + s.length() + " > " + screenWidth);
         }
       } else {
-        if (s.length() > 50 && s.length() <= width) {
-          fail("Expected length <= " + width + " is " + s.length());
+        if (s.length() != 0 && s.length() <= screenWidth) {
+          fail("Expected length <= screenWidth: " + s.length() + " <= " + screenWidth);
         }
       }
-
     }
+
+    return lines;
   }
 
   /**
-   * Test Variations tablewide separator true false
+   * Test Variations table-wide separator true false
    */
   @Test
-  public void testBasicScraping() throws ClassNotFoundException, IOException {
-    Properties props = new Properties();
-    props.setProperty(Gfsh.ENV_APP_RESULT_VIEWER, Gfsh.DEFAULT_APP_RESULT_VIEWER);
-    createShell(props);
+  public void testSanity() throws Exception {
     assertTrue(TableBuilderHelper.shouldTrimColumns());
-    doTableBuilderTestUnit(15, 4, "|", true, false);
+
+    Table table = createTableStructure(3, "|");
+    RowGroup rowGroup = table.getLastRowGroup();
+    Row row1 = rowGroup.newRow();
+    row1.newLeftCol("1")
+        .newLeftCol("1")
+        .newLeftCol("1");
+
+    List<String> result = validateTable(table, true);
+    // Check the last line
+    assertEquals("1     |1     |1", result.get(3));
   }
 
+  @Test
+  public void testLastColumnTruncated() throws Exception {
+    assertTrue(TableBuilderHelper.shouldTrimColumns());
+
+    Table table = createTableStructure(4, "|");
+    RowGroup rowGroup = table.getLastRowGroup();
+    Row row1 = rowGroup.newRow();
+    row1.newLeftCol("1")
+        .newLeftCol("123456789-")
+        .newLeftCol("123456789-")
+        .newLeftCol("123456789-123456789-12345");
+
+    List<String> result = validateTable(table, true);
+    // Check the last line
+    assertEquals("1     |123456789-|123456789-|123456789..", result.get(3));
+  }
 
   @Test
-  public void testSeparatorWithMultipleChars() throws ClassNotFoundException, IOException {
-    Properties props = new Properties();
-    props.setProperty(Gfsh.ENV_APP_RESULT_VIEWER, Gfsh.DEFAULT_APP_RESULT_VIEWER);
-    createShell(props);
+  public void testLongestColumnFirstTruncated() throws Exception {
     assertTrue(TableBuilderHelper.shouldTrimColumns());
-    doTableBuilderTestUnit(15, 4, " | ", true, false);
+
+    Table table = createTableStructure(4, "|");
+    RowGroup rowGroup = table.getLastRowGroup();
+    Row row1 = rowGroup.newRow();
+    row1.newLeftCol("123456789-123456789-")
+        .newLeftCol("123456789-12345")
+        .newLeftCol("123456789-")
+        .newLeftCol("1");
+
+    List<String> result = validateTable(table, true);
+    // Check the last line
+    assertEquals("1234..|123456789-12345|123456789-|1", result.get(3));
+  }
+
+  @Test
+  public void testMultipleColumnsTruncated() throws Exception {
+    assertTrue(TableBuilderHelper.shouldTrimColumns());
+
+    Table table = createTableStructure(4, "|");
+    RowGroup rowGroup = table.getLastRowGroup();
+    Row row1 = rowGroup.newRow();
+    row1.newLeftCol("1")
+        .newLeftCol("123456789-")
+        .newLeftCol("123456789-123456789-123456789-")
+        .newLeftCol("123456789-123456789-12345");
+
+    List<String> result = validateTable(table, true);
+    // Check the last line
+    assertEquals("1     |123456789-|123456789..|1234567..", result.get(3));
+  }
+
+  @Test
+  public void testMultipleColumnsTruncatedLongestFirst() throws Exception {
+    assertTrue(TableBuilderHelper.shouldTrimColumns());
+
+    Table table = createTableStructure(4, "|");
+    RowGroup rowGroup = table.getLastRowGroup();
+    Row row1 = rowGroup.newRow();
+    row1.newLeftCol("123456789-123456789-123456789-")
+        .newLeftCol("123456789-123456789-12345")
+        .newLeftCol("1")
+        .newLeftCol("123456789-");
+
+    List<String> result = validateTable(table, true);
+    // Check the last line
+    assertEquals("123456789..|1234567..|1     |123456789-", result.get(3));
+  }
+
+  @Test
+  public void testColumnsWithShortNames() throws Exception {
+    when(TableBuilderHelper.class, "getScreenWidth").thenReturn(9);
+    assertTrue(TableBuilderHelper.shouldTrimColumns());
+
+    Table table = createTableStructure(3, "|", "A", "A", "A");
+    RowGroup rowGroup = table.getLastRowGroup();
+    Row row1 = rowGroup.newRow();
+    row1.newLeftCol("123")
+        .newLeftCol("123")
+        .newLeftCol("123");
+
+    List<String> result = validateTable(table, true);
+    // Check the last line
+    assertEquals("..|..|..", result.get(3));
+  }
+
+  @Test(expected = TableBuilderHelper.TooManyColumnsException.class)
+  public void testExceptionTooSmallWidth() throws Exception {
+    when(TableBuilderHelper.class, "getScreenWidth").thenReturn(7);
+    assertTrue(TableBuilderHelper.shouldTrimColumns());
+
+    Table table = createTableStructure(3, "|", "A", "A", "A");
+    RowGroup rowGroup = table.getLastRowGroup();
+    Row row1 = rowGroup.newRow();
+    row1.newLeftCol("12")
+        .newLeftCol("12")
+        .newLeftCol("12");
+
+    // This should throw an exception
+    List<String> result = validateTable(table, true);
+  }
+
+  @Test
+  public void testTooLittleSpaceOnNextToLastColumn() throws Exception {
+    assertTrue(TableBuilderHelper.shouldTrimColumns());
+
+    Table table = createTableStructure(4, "|");
+    RowGroup rowGroup = table.getLastRowGroup();
+    Row row1 = rowGroup.newRow();
+    row1.newLeftCol("1")
+        .newLeftCol("123456789-")
+        .newLeftCol("123456789-123456789-123456789-")
+        .newLeftCol("123456789-123456789-12345");
+
+    List<String> result = validateTable(table, true);
+    // Check the last line
+    assertEquals("1     |123456789-|123456789..|1234567..", result.get(3));
+  }
+
+  @Test
+  public void testSeparatorWithMultipleChars() throws Exception {
+    assertTrue(TableBuilderHelper.shouldTrimColumns());
+
+    Table table = createTableStructure(4, "<|>");
+    RowGroup rowGroup = table.getLastRowGroup();
+    Row row1 = rowGroup.newRow();
+    row1.newLeftCol("1")
+        .newLeftCol("123456789-")
+        .newLeftCol("123456789-")
+        .newLeftCol("123456789-123456789-12345");
+
+    List<String> result = validateTable(table, true);
+    // Check the last line
+    assertEquals("1     <|>123456789-<|>123456789-<|>123..", result.get(3));
   }
 
   /**
    * multiple columns upto 8 : done
    */
   @Test
-  @Ignore("Bug 52051")
-  public void testManyColumns() throws ClassNotFoundException, IOException {
-    createShell(null);
+  public void testManyColumns() throws Exception {
     assertTrue(TableBuilderHelper.shouldTrimColumns());
-    doTableBuilderTestUnit(15, 6, "|", true, true);
+
+    Table table = createTableStructure(8, "|");
+    RowGroup rowGroup = table.getLastRowGroup();
+    Row row1 = rowGroup.newRow();
+    row1.newLeftCol("123456789-")
+        .newLeftCol("123456789-")
+        .newLeftCol("123456789-")
+        .newLeftCol("123456789-")
+        .newLeftCol("123456789-")
+        .newLeftCol("123456789-")
+        .newLeftCol("123456789-")
+        .newLeftCol("123456789-");
+
+    List<String> result = validateTable(table, true);
+    // Check the last line
+    assertEquals("123456789-|123456789-|..|..|..|..|..|..", result.get(3));
   }
 
   /**
    * set gfsh env property result_viewer to basic disable for external reader
    */
-  //
   @Test
-  public void testDisableColumnAdjustment() throws ClassNotFoundException, IOException {
-    createShell(null);
+  public void testDisableColumnAdjustment() throws Exception {
+    when(TableBuilderHelper.class, "shouldTrimColumns").thenReturn(false);
     assertFalse(TableBuilderHelper.shouldTrimColumns());
-    doTableBuilderTestUnit(15, 12, "|", false, false);
-  }
 
+    Table table = createTableStructure(5, "|");
+    RowGroup rowGroup = table.getLastRowGroup();
+    Row row1 = rowGroup.newRow();
+    row1.newLeftCol("1")
+        .newLeftCol("123456789-")
+        .newLeftCol("123456789-")
+        .newLeftCol("123456789-123456789-12345")
+        .newLeftCol("1");
+
+    List<String> result = validateTable(table, false);
+    // Check the last line
+    assertEquals("1     |123456789-|123456789-|123456789-123456789-12345|1", result.get(3));
+  }
 }

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/c19c3cab/gradle/dependency-versions.properties
----------------------------------------------------------------------
diff --git a/gradle/dependency-versions.properties b/gradle/dependency-versions.properties
index 71cfd43..30a7fd8 100644
--- a/gradle/dependency-versions.properties
+++ b/gradle/dependency-versions.properties
@@ -44,6 +44,7 @@ hbase.version = 0.94.27
 jackson.version = 2.2.0
 jackson-module-scala_2.10.version = 2.1.5
 jansi.version = 1.8
+javassist.version = 3.20.0-GA
 javax.mail-api.version = 1.4.5
 javax.resource-api.version = 1.7
 javax.servlet-api.version = 3.1.0
@@ -66,6 +67,7 @@ mx4j-tools.version = 3.0.1
 netty-all.version = 4.0.4.Final
 objenesis.version = 2.1
 paranamer.version = 2.3
+powermock.version = 1.6.4
 quartz.version = 2.2.1
 scala.version = 2.10.0
 slf4j-api.version = 1.7.7