You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zeppelin.apache.org by mo...@apache.org on 2017/03/01 02:14:55 UTC

zeppelin git commit: [ZEPPELIN-2155] [HOTFIX] Flaky test: NotebookTest

Repository: zeppelin
Updated Branches:
  refs/heads/master a26bd2d76 -> 336df5617


[ZEPPELIN-2155] [HOTFIX] Flaky test: NotebookTest

### What is this PR for?
This PR fixes test failure described in ZEPPELIN-2155.
All the PR test fails with this problem at the moment.

Problem came from https://github.com/apache/zeppelin/pull/2049, which fixes wrong authentication configuration for the test. And previously test were written for wrong configuration.

This PR fixes wrong test and remove some assert statement that may occur flaky test but don't really verify anything.

### What type of PR is it?
Hot Fix

### Todos
* [x] - fix problem

### What is the Jira issue?

### How should this be tested?
CI green

### Questions:
* Does the licenses files need update? no
* Is there breaking changes for older versions? no
* Does this needs documentation? no

Author: Lee moon soo <mo...@apache.org>

Closes #2081 from Leemoonsoo/ZEPPELIN-2155 and squashes the following commits:

0360108 [Lee moon soo] Correct test implementation with Authentication Enable
21d91ef [Lee moon soo] Handle multiple Set-Cookie headers
0ff3ff7 [Lee moon soo] Turn off Authentication on DirAccessTest because it is not necessary
52266a2 [Lee moon soo] Remove unnecessary assert


Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo
Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/336df561
Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/336df561
Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/336df561

Branch: refs/heads/master
Commit: 336df5617b3a2ca43a8fe7b600508d8e8b0b9a25
Parents: a26bd2d
Author: Lee moon soo <mo...@apache.org>
Authored: Tue Feb 28 21:13:12 2017 +0900
Committer: Lee moon soo <mo...@apache.org>
Committed: Wed Mar 1 11:14:47 2017 +0900

----------------------------------------------------------------------
 .../org/apache/zeppelin/rest/AbstractTestRestApi.java  | 13 ++++++++++---
 .../org/apache/zeppelin/rest/SecurityRestApiTest.java  | 13 ++++++-------
 .../org/apache/zeppelin/security/DirAccessTest.java    |  4 ++--
 .../org/apache/zeppelin/notebook/NotebookTest.java     |  5 ++---
 4 files changed, 20 insertions(+), 15 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/zeppelin/blob/336df561/zeppelin-server/src/test/java/org/apache/zeppelin/rest/AbstractTestRestApi.java
----------------------------------------------------------------------
diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/AbstractTestRestApi.java b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/AbstractTestRestApi.java
index 99048ee..7ea2774 100644
--- a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/AbstractTestRestApi.java
+++ b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/AbstractTestRestApi.java
@@ -31,6 +31,7 @@ import java.util.regex.Pattern;
 import org.apache.commons.exec.CommandLine;
 import org.apache.commons.exec.DefaultExecutor;
 import org.apache.commons.exec.PumpStreamHandler;
+import org.apache.commons.httpclient.Header;
 import org.apache.commons.httpclient.HttpClient;
 import org.apache.commons.httpclient.HttpMethodBase;
 import org.apache.commons.httpclient.cookie.CookiePolicy;
@@ -333,7 +334,7 @@ public abstract class AbstractTestRestApi {
     GetMethod request = null;
     boolean isRunning = true;
     try {
-      request = httpGet("/");
+      request = httpGet("/version");
       isRunning = request.getStatusCode() == 200;
     } catch (IOException e) {
       LOG.error("AbstractTestRestApi.checkIfServerIsRunning() fails .. ZeppelinServer is not running");
@@ -427,8 +428,14 @@ public abstract class AbstractTestRestApi {
     httpClient.executeMethod(postMethod);
     LOG.info("{} - {}", postMethod.getStatusCode(), postMethod.getStatusText());
     Pattern pattern = Pattern.compile("JSESSIONID=([a-zA-Z0-9-]*)");
-    java.util.regex.Matcher matcher = pattern.matcher(postMethod.getResponseHeaders("Set-Cookie")[0].toString());
-    return matcher.find()? matcher.group(1) : StringUtils.EMPTY;
+    Header[] setCookieHeaders = postMethod.getResponseHeaders("Set-Cookie");
+    for (Header setCookie : setCookieHeaders) {
+      java.util.regex.Matcher matcher = pattern.matcher(setCookie.toString());
+      if (matcher.find()) {
+        return matcher.group(1);
+      }
+    }
+    return StringUtils.EMPTY;
   }
 
   protected static boolean userAndPasswordAreNotBlank(String user, String pwd) {

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/336df561/zeppelin-server/src/test/java/org/apache/zeppelin/rest/SecurityRestApiTest.java
----------------------------------------------------------------------
diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/SecurityRestApiTest.java b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/SecurityRestApiTest.java
index b56763a..bc38f74 100644
--- a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/SecurityRestApiTest.java
+++ b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/SecurityRestApiTest.java
@@ -40,7 +40,7 @@ public class SecurityRestApiTest extends AbstractTestRestApi {
 
   @BeforeClass
   public static void init() throws Exception {
-    AbstractTestRestApi.startUpWithAuthenticationEnable();;
+    AbstractTestRestApi.startUpWithAuthenticationEnable();
   }
 
   @AfterClass
@@ -50,21 +50,21 @@ public class SecurityRestApiTest extends AbstractTestRestApi {
 
   @Test
   public void testTicket() throws IOException {
-    GetMethod get = httpGet("/security/ticket");
+    GetMethod get = httpGet("/security/ticket", "admin", "password1");
     get.addRequestHeader("Origin", "http://localhost");
     Map<String, Object> resp = gson.fromJson(get.getResponseBodyAsString(),
         new TypeToken<Map<String, Object>>(){}.getType());
     Map<String, String> body = (Map<String, String>) resp.get("body");
     collector.checkThat("Paramater principal", body.get("principal"),
-        CoreMatchers.equalTo("anonymous"));
+        CoreMatchers.equalTo("admin"));
     collector.checkThat("Paramater ticket", body.get("ticket"),
-        CoreMatchers.equalTo("anonymous"));
+        CoreMatchers.not("anonymous"));
     get.releaseConnection();
   }
 
   @Test
   public void testGetUserList() throws IOException {
-    GetMethod get = httpGet("/security/userlist/admi");
+    GetMethod get = httpGet("/security/userlist/admi", "admin", "password1");
     get.addRequestHeader("Origin", "http://localhost");
     Map<String, Object> resp = gson.fromJson(get.getResponseBodyAsString(),
         new TypeToken<Map<String, Object>>(){}.getType());
@@ -75,7 +75,7 @@ public class SecurityRestApiTest extends AbstractTestRestApi {
         CoreMatchers.equalTo(true));
     get.releaseConnection();
 
-    GetMethod notUser = httpGet("/security/userlist/randomString");
+    GetMethod notUser = httpGet("/security/userlist/randomString", "admin", "password1");
     notUser.addRequestHeader("Origin", "http://localhost");
     Map<String, Object> notUserResp = gson.fromJson(notUser.getResponseBodyAsString(),
         new TypeToken<Map<String, Object>>(){}.getType());
@@ -85,6 +85,5 @@ public class SecurityRestApiTest extends AbstractTestRestApi {
 
     notUser.releaseConnection();
   }
-
 }
 

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/336df561/zeppelin-server/src/test/java/org/apache/zeppelin/security/DirAccessTest.java
----------------------------------------------------------------------
diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/security/DirAccessTest.java b/zeppelin-server/src/test/java/org/apache/zeppelin/security/DirAccessTest.java
index 7837573..2f2486b 100644
--- a/zeppelin-server/src/test/java/org/apache/zeppelin/security/DirAccessTest.java
+++ b/zeppelin-server/src/test/java/org/apache/zeppelin/security/DirAccessTest.java
@@ -30,7 +30,7 @@ public class DirAccessTest extends AbstractTestRestApi {
   public void testDirAccessForbidden() throws Exception {
     synchronized (this) {
       System.setProperty(ZeppelinConfiguration.ConfVars.ZEPPELIN_SERVER_DEFAULT_DIR_ALLOWED.getVarName(), "false");
-      AbstractTestRestApi.startUpWithAuthenticationEnable();
+      AbstractTestRestApi.startUp();
       HttpClient httpClient = new HttpClient();
       GetMethod getMethod = new GetMethod(getUrlToTest() + "/app/");
       httpClient.executeMethod(getMethod);
@@ -43,7 +43,7 @@ public class DirAccessTest extends AbstractTestRestApi {
   public void testDirAccessOk() throws Exception {
     synchronized (this) {
       System.setProperty(ZeppelinConfiguration.ConfVars.ZEPPELIN_SERVER_DEFAULT_DIR_ALLOWED.getVarName(), "true");
-      AbstractTestRestApi.startUpWithAuthenticationEnable();
+      AbstractTestRestApi.startUp();
       HttpClient httpClient = new HttpClient();
       GetMethod getMethod = new GetMethod(getUrlToTest() + "/app/");
       httpClient.executeMethod(getMethod);

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/336df561/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java
index 50775c0..6799410 100644
--- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java
+++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java
@@ -1159,7 +1159,7 @@ public class NotebookTest implements JobListenerFactory{
     assertEquals(notebookAuthorization.getOwners(notePublic.getId()).size(), 1);
     assertEquals(notebookAuthorization.getReaders(notePublic.getId()).size(), 0);
     assertEquals(notebookAuthorization.getWriters(notePublic.getId()).size(), 0);
-    
+
     // case of private note
     System.setProperty(ConfVars.ZEPPELIN_NOTEBOOK_PUBLIC.getVarName(), "false");
     ZeppelinConfiguration conf2 = ZeppelinConfiguration.create();
@@ -1181,8 +1181,7 @@ public class NotebookTest implements JobListenerFactory{
     notes2 = notebook.getAllNotes(user2);
     assertEquals(notes1.size(), 2);
     assertEquals(notes2.size(), 1);
-    assertEquals(notes1.get(1).getId(), notePrivate.getId());
-    
+
     // user1 have all rights
     assertEquals(notebookAuthorization.getOwners(notePrivate.getId()).size(), 1);
     assertEquals(notebookAuthorization.getReaders(notePrivate.getId()).size(), 1);