You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by ad...@apache.org on 2017/10/10 00:54:52 UTC

kudu git commit: java client: fix TestKuduTable.testAlterNoWait

Repository: kudu
Updated Branches:
  refs/heads/master 1d928951e -> 1680d23be


java client: fix TestKuduTable.testAlterNoWait

There were a couple issues here, some cosmetic and some real:
- The first block comment incorrectly referred to oldName instead of
  newName as the column being checked.
- After newName was found to be non-existent, the second check should have
  reused the same KuduTable and should have checked oldName, but it checked
  newName again, expecting it to exist. That adds no value and makes the
  test flaky (i.e. if the alter finishes in between, the test fails).
- In the check after the call to isAlterTableDone, the fail() message was
  backwards; oldName should not exist at this point.

Change-Id: I498413ad118631eebda2d16d6c22727409214896
Reviewed-on: http://gerrit.cloudera.org:8080/8213
Tested-by: Adar Dembo <ad...@cloudera.com>
Reviewed-by: Dan Burkert <da...@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/1680d23b
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/1680d23b
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/1680d23b

Branch: refs/heads/master
Commit: 1680d23bee0a3c95e20a7b84a52a075d06af1e6c
Parents: 1d92895
Author: Adar Dembo <ad...@cloudera.com>
Authored: Wed Oct 4 15:56:06 2017 -0700
Committer: Adar Dembo <ad...@cloudera.com>
Committed: Tue Oct 10 00:54:38 2017 +0000

----------------------------------------------------------------------
 .../org/apache/kudu/client/TestKuduTable.java   | 34 +++++++++++---------
 1 file changed, 18 insertions(+), 16 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/1680d23b/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java b/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java
index 401f8be..b07f45a 100644
--- a/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java
+++ b/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java
@@ -587,31 +587,33 @@ public class TestKuduTable extends BaseKuduTest {
           .renameColumn(oldName, newName)
           .setWait(false));
 
-      // Reload the schema and test for the existence of 'oldName'. Since we
-      // didn't wait for alter to finish, we should be able to find it. However,
-      // this is timing dependent: if the alter finishes before we reload the
-      // schema, loop and try again.
+      // We didn't wait for the column rename to finish, so we should be able
+      // to still see 'oldName' and not yet see 'newName'. However, this is
+      // timing dependent: if the alter finishes before we reload the schema,
+      // loop and try again.
+      KuduTable table = syncClient.openTable(tableName);
       try {
-        syncClient.openTable(tableName).getSchema().getColumn(newName);
-        LOG.info("Alter finished too quickly (new column name {} is already " +
-            "visible), trying again", oldName);
+        table.getSchema().getColumn(oldName);
+      } catch (IllegalArgumentException e) {
+        LOG.info("Alter finished too quickly (old column name {} is already " +
+            "gone), trying again", oldName);
         oldName = newName;
         continue;
-      } catch (IllegalArgumentException e) {}
-
+      }
       try {
-        syncClient.openTable(tableName).getSchema().getColumn(newName);
-        fail(String.format("New column name %s should have been visible", newName));
+        table.getSchema().getColumn(newName);
+        fail(String.format("New column name %s should not yet be visible", newName));
       } catch (IllegalArgumentException e) {}
 
       // After waiting for the alter to finish and reloading the schema,
-      // the new column name should be visible.
-      syncClient.isAlterTableDone(tableName);
+      // 'newName' should be visible and 'oldName' should be gone.
+      assertTrue(syncClient.isAlterTableDone(tableName));
+      table = syncClient.openTable(tableName);
       try {
-        syncClient.openTable(tableName).getSchema().getColumn(oldName);
-        fail(String.format("Old column name %s should have been visible", oldName));
+        table.getSchema().getColumn(oldName);
+        fail(String.format("Old column name %s should not be visible", oldName));
       } catch (IllegalArgumentException e) {}
-      syncClient.openTable(tableName).getSchema().getColumn(newName);
+      table.getSchema().getColumn(newName);
       LOG.info("Test passed on attempt {}", i + 1);
       return;
     }