zeppelin-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From b..@apache.org
Subject zeppelin git commit: ZEPPELIN-1063: fix flaky python interpreter test
Date Tue, 28 Jun 2016 10:07:50 GMT
Repository: zeppelin
Updated Branches:
  refs/heads/master a7bb45393 -> 7951e6c69


ZEPPELIN-1063: fix flaky python interpreter test

### What is this PR for?
fix flaky python interpreter test

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

### Todos
* [x] cleanup test code
* [x] fix flaky open port check

### What is the Jira issue?
[ZEPPELIN-1063](https://issues.apache.org/jira/browse/ZEPPELIN-1063)

### How should this be tested?
`mvn "-Dtest=org.apache.zeppelin.python.PythonInterpreterTest" test -pl python`

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

Author: Alexander Bezzubov <bzz@apache.org>

Closes #1094 from bzz/fix/python-tests/ZEPPELIN-1063 and squashes the following commits:

b9de5a2 [Alexander Bezzubov] Python: refactoring open port checking
09f3018 [Alexander Bezzubov] Python: refactoring - arrange imports
46e42e8 [Alexander Bezzubov] Python: refactoring mock() structure and JavaDocs
651d8e8 [Alexander Bezzubov] Python: refactoring loggers
d7f8cdd [Alexander Bezzubov] Python: normalize newlines in tests


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

Branch: refs/heads/master
Commit: 7951e6c698aa2e009c461488c3231e23272e08bc
Parents: a7bb453
Author: Alexander Bezzubov <bzz@apache.org>
Authored: Mon Jun 27 20:43:09 2016 +0900
Committer: Alexander Bezzubov <bzz@apache.org>
Committed: Tue Jun 28 19:07:42 2016 +0900

----------------------------------------------------------------------
 .../zeppelin/python/PythonInterpreter.java      |  53 +++---
 .../zeppelin/python/PythonInterpreterTest.java  | 161 ++++++++-----------
 2 files changed, 96 insertions(+), 118 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/zeppelin/blob/7951e6c6/python/src/main/java/org/apache/zeppelin/python/PythonInterpreter.java
----------------------------------------------------------------------
diff --git a/python/src/main/java/org/apache/zeppelin/python/PythonInterpreter.java b/python/src/main/java/org/apache/zeppelin/python/PythonInterpreter.java
index 08bb944..06e0c99 100644
--- a/python/src/main/java/org/apache/zeppelin/python/PythonInterpreter.java
+++ b/python/src/main/java/org/apache/zeppelin/python/PythonInterpreter.java
@@ -17,6 +17,14 @@
 
 package org.apache.zeppelin.python;
 
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.io.InputStreamReader;
+import java.net.ServerSocket;
+import java.util.Collection;
+import java.util.List;
+import java.util.Properties;
+
 import org.apache.zeppelin.display.GUI;
 import org.apache.zeppelin.interpreter.Interpreter;
 import org.apache.zeppelin.interpreter.InterpreterContext;
@@ -29,21 +37,14 @@ import org.apache.zeppelin.scheduler.SchedulerFactory;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import py4j.GatewayServer;
 
-import java.io.BufferedReader;
-import java.io.IOException;
-import java.io.InputStreamReader;
-import java.net.ServerSocket;
-import java.util.Collection;
-import java.util.List;
-import java.util.Properties;
+import py4j.GatewayServer;
 
 /**
  * Python interpreter for Zeppelin.
  */
 public class PythonInterpreter extends Interpreter {
-  Logger logger = LoggerFactory.getLogger(PythonInterpreter.class);
+  private static final Logger LOG = LoggerFactory.getLogger(PythonInterpreter.class);
 
   public static final String BOOTSTRAP_PY = "/bootstrap.py";
   public static final String BOOTSTRAP_INPUT_PY = "/bootstrap_input.py";
@@ -65,8 +66,8 @@ public class PythonInterpreter extends Interpreter {
 
   @Override
   public void open() {
-    logger.info("Starting Python interpreter .....");
-    logger.info("Python path is set to:" + property.getProperty(ZEPPELIN_PYTHON));
+    LOG.info("Starting Python interpreter .....");
+    LOG.info("Python path is set to:" + property.getProperty(ZEPPELIN_PYTHON));
 
     maxResult = Integer.valueOf(getProperty(MAX_RESULT));
     process = getPythonProcess();
@@ -74,32 +75,32 @@ public class PythonInterpreter extends Interpreter {
     try {
       process.open();
     } catch (IOException e) {
-      logger.error("Can't start the python process", e);
+      LOG.error("Can't start the python process", e);
     }
 
     try {
-      logger.info("python PID : " + process.getPid());
+      LOG.info("python PID : " + process.getPid());
     } catch (Exception e) {
-      logger.warn("Can't find python pid process", e);
+      LOG.warn("Can't find python pid process", e);
     }
 
     try {
-      logger.info("Bootstrap interpreter with " + BOOTSTRAP_PY);
+      LOG.info("Bootstrap interpreter with " + BOOTSTRAP_PY);
       bootStrapInterpreter(BOOTSTRAP_PY);
     } catch (IOException e) {
-      logger.error("Can't execute " + BOOTSTRAP_PY + " to initiate python process", e);
+      LOG.error("Can't execute " + BOOTSTRAP_PY + " to initiate python process", e);
     }
 
     if (py4J = isPy4jInstalled()) {
       port = findRandomOpenPortOnAllLocalInterfaces();
-      logger.info("Py4j gateway port : " + port);
+      LOG.info("Py4j gateway port : " + port);
       try {
         gatewayServer = new GatewayServer(this, port);
         gatewayServer.start();
-        logger.info("Bootstrap inputs with " + BOOTSTRAP_INPUT_PY);
+        LOG.info("Bootstrap inputs with " + BOOTSTRAP_INPUT_PY);
         bootStrapInterpreter(BOOTSTRAP_INPUT_PY);
       } catch (IOException e) {
-        logger.error("Can't execute " + BOOTSTRAP_INPUT_PY + " to " +
+        LOG.error("Can't execute " + BOOTSTRAP_INPUT_PY + " to " +
             "initialize Zeppelin inputs in python process", e);
       }
     }
@@ -107,7 +108,7 @@ public class PythonInterpreter extends Interpreter {
 
   @Override
   public void close() {
-    logger.info("closing Python interpreter .....");
+    LOG.info("closing Python interpreter .....");
     try {
       if (process != null) {
         process.close();
@@ -116,7 +117,7 @@ public class PythonInterpreter extends Interpreter {
         gatewayServer.shutdown();
       }
     } catch (IOException e) {
-      logger.error("Can't close the interpreter", e);
+      LOG.error("Can't close the interpreter", e);
     }
   }
 
@@ -136,7 +137,7 @@ public class PythonInterpreter extends Interpreter {
     try {
       process.interrupt();
     } catch (IOException e) {
-      logger.error("Can't interrupt the python interpreter", e);
+      LOG.error("Can't interrupt the python interpreter", e);
     }
   }
 
@@ -184,11 +185,11 @@ public class PythonInterpreter extends Interpreter {
 
   private String sendCommandToPython(String cmd) {
     String output = "";
-    logger.info("Sending : \n" + (cmd.length() > 200 ? cmd.substring(0, 200) + "..." :
cmd));
+    LOG.info("Sending : \n" + (cmd.length() > 200 ? cmd.substring(0, 200) + "..." : cmd));
     try {
       output = process.sendAndGetResult(cmd);
     } catch (IOException e) {
-      logger.error("Error when sending commands to python process", e);
+      LOG.error("Error when sending commands to python process", e);
     }
     //logger.info("Got : \n" + output);
     return output;
@@ -207,7 +208,7 @@ public class PythonInterpreter extends Interpreter {
     if (py4J && port != null && port != -1) {
       bootstrapCode = bootstrapCode.replaceAll("\\%PORT\\%", port.toString());
     }
-    logger.info("Bootstrap python interpreter with code from \n " + file);
+    LOG.info("Bootstrap python interpreter with code from \n " + file);
     sendCommandToPython(bootstrapCode);
   }
 
@@ -234,7 +235,7 @@ public class PythonInterpreter extends Interpreter {
       port = socket.getLocalPort();
       socket.close();
     } catch (IOException e) {
-      logger.error("Can't find an open port", e);
+      LOG.error("Can't find an open port", e);
     }
     return port;
   }

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/7951e6c6/python/src/test/java/org/apache/zeppelin/python/PythonInterpreterTest.java
----------------------------------------------------------------------
diff --git a/python/src/test/java/org/apache/zeppelin/python/PythonInterpreterTest.java b/python/src/test/java/org/apache/zeppelin/python/PythonInterpreterTest.java
index 2e8060d..92fde2d 100644
--- a/python/src/test/java/org/apache/zeppelin/python/PythonInterpreterTest.java
+++ b/python/src/test/java/org/apache/zeppelin/python/PythonInterpreterTest.java
@@ -18,22 +18,26 @@
 package org.apache.zeppelin.python;
 
 import static org.apache.zeppelin.python.PythonInterpreter.*;
-
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
-import static org.mockito.Mockito.eq;
-import static org.mockito.Mockito.times;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Matchers.anyString;
+import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
 import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.anyString;
+import static org.mockito.Mockito.when;
 
-import org.apache.zeppelin.interpreter.InterpreterResult;
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.net.Socket;
+import java.net.SocketAddress;
+import java.util.Properties;
 
+import org.apache.zeppelin.interpreter.InterpreterResult;
 import org.junit.Before;
 import org.junit.Test;
 import org.mockito.invocation.InvocationOnMock;
@@ -41,18 +45,11 @@ import org.mockito.stubbing.Answer;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.io.IOException;
-import java.net.InetSocketAddress;
-import java.net.Socket;
-import java.net.SocketAddress;
-import java.util.Properties;
-
 /**
  * Python interpreter unit test
  */
 public class PythonInterpreterTest {
-
-  Logger logger = LoggerFactory.getLogger(PythonProcess.class);
+  private static final Logger LOG = LoggerFactory.getLogger(PythonProcess.class);
 
   PythonInterpreter pythonInterpreter = null;
   PythonProcess mockPythonProcess;
@@ -66,75 +63,54 @@ public class PythonInterpreterTest {
   }
 
   @Before
-  public void beforeTest() {
+  public void beforeTest() throws IOException {
     cmdHistory = "";
 
     /*Mock python process*/
     mockPythonProcess = mock(PythonProcess.class);
-    when(mockPythonProcess.getPid()).thenReturn((long) 1);
-    try {
-      when(mockPythonProcess.sendAndGetResult(anyString())).thenAnswer(
-          new Answer<String>() {
-        @Override
-        public String answer(InvocationOnMock invocationOnMock) throws Throwable {
-          return answerFromPythonMock(invocationOnMock);
-        }
-      });
-    } catch (IOException e) {
-      logger.error("Can't initiate python process", e);
-    }
+    when(mockPythonProcess.getPid()).thenReturn(1L);
+    when(mockPythonProcess.sendAndGetResult(anyString())).thenAnswer(new Answer<String>()
{
+      @Override public String answer(InvocationOnMock invocationOnMock) throws Throwable
{
+        return answerFromPythonMock(invocationOnMock);
+      }
+    });
 
     pythonInterpreter = spy(new PythonInterpreter(getPythonTestProperties()));
 
     when(pythonInterpreter.getPythonProcess()).thenReturn(mockPythonProcess);
-
-    try {
-      when(mockPythonProcess.sendAndGetResult(eq("\n\nimport py4j\n"))).thenReturn("ImportError");
-    } catch (IOException e) {
-      e.printStackTrace();
-    }
+    when(mockPythonProcess.sendAndGetResult(eq("\n\nimport py4j\n"))).thenReturn("ImportError");
   }
 
   @Test
   public void testOpenInterpreter() {
     pythonInterpreter.open();
     assertEquals(pythonInterpreter.getPythonProcess().getPid(), 1);
-
   }
 
-  @Test
-  public void testPy4jIsNotInstalled() {
-
-    /*
-    If Py4J is not installed, bootstrap_input.py
-    is not sent to Python process and
-    py4j JavaGateway is not running
-     */
+  /**
+   * If Py4J is not installed, bootstrap_input.py
+   * is not sent to Python process and py4j JavaGateway is not running
+   */
+  @Test public void testPy4jIsNotInstalled() {
     pythonInterpreter.open();
     assertNull(pythonInterpreter.getPy4jPort());
-
     assertTrue(cmdHistory.contains("def help()"));
     assertTrue(cmdHistory.contains("class PyZeppelinContext(object):"));
     assertTrue(cmdHistory.contains("z = PyZeppelinContext"));
     assertTrue(cmdHistory.contains("def show"));
     assertFalse(cmdHistory.contains("GatewayClient"));
-
   }
 
-  @Test
-  public void testPy4jInstalled() {
-
-    /*
-    If Py4J installed, bootstrap_input.py
-    is sent to interpreter and JavaGateway is
-    running
-     */
+  /**
+   * If Py4J installed, bootstrap_input.py
+   * is sent to interpreter and JavaGateway is
+   * running
+   *
+   * @throws IOException
+   */
+  @Test public void testPy4jInstalled() throws IOException {
+    when(mockPythonProcess.sendAndGetResult(eq("\n\nimport py4j\n"))).thenReturn(">>>");
 
-    try {
-      when(mockPythonProcess.sendAndGetResult(eq("\n\nimport py4j\n"))).thenReturn(">>>");
-    } catch (IOException e) {
-      e.printStackTrace();
-    }
     pythonInterpreter.open();
     Integer py4jPort = pythonInterpreter.getPy4jPort();
     assertNotNull(py4jPort);
@@ -146,70 +122,71 @@ public class PythonInterpreterTest {
     assertTrue(cmdHistory.contains("GatewayClient(port=" + py4jPort + ")"));
     assertTrue(cmdHistory.contains("org.apache.zeppelin.display.Input"));
 
-    assertTrue(checkSocketAddress(py4jPort));
-
+    assertTrue(serverIsListeningOn(py4jPort));
   }
 
-
   @Test
-  public void testClose() {
+  public void testClose() throws IOException, InterruptedException {
+    //given: py4j is installed
+    when(mockPythonProcess.sendAndGetResult(eq("\n\nimport py4j\n"))).thenReturn(">>>");
 
-    try {
-      when(mockPythonProcess.sendAndGetResult(eq("\n\nimport py4j\n"))).thenReturn(">>>");
-    } catch (IOException e) {
-      e.printStackTrace();
-    }
     pythonInterpreter.open();
     Integer py4jPort = pythonInterpreter.getPy4jPort();
-
     assertNotNull(py4jPort);
+
+    //when
     pythonInterpreter.close();
 
-    assertFalse(checkSocketAddress(py4jPort));
-    try {
-      verify(mockPythonProcess, times(1)).close();
-    } catch (IOException e) {
-      e.printStackTrace();
-    }
+    //then
+    assertFalse(serverIsListeningOn(py4jPort));
+    verify(mockPythonProcess, times(1)).close();
   }
 
-
   @Test
   public void testInterpret() {
-
     pythonInterpreter.open();
     cmdHistory = "";
     InterpreterResult result = pythonInterpreter.interpret("print a", null);
     assertEquals(InterpreterResult.Code.SUCCESS, result.code());
     assertEquals("%text print a", result.toString());
-
   }
 
+  /**
+   * Checks if given port is open on 'localhost'
+   * @param port
+   */
+  private boolean serverIsListeningOn(Integer port) {
+    boolean serverIsListening = false;
+    Socket s = new Socket();
 
+    boolean connected = tryToConnect(s, port);
+    if (connected) {
+      serverIsListening = true;
+      tryToClose(s);
+    }
+    return serverIsListening;
+  }
 
-  private boolean checkSocketAddress(Integer py4jPort) {
-    Socket s = new Socket();
-    SocketAddress sa = new InetSocketAddress("localhost", py4jPort);
-    Boolean working = null;
+  private boolean tryToConnect(Socket s, Integer port) {
+    boolean connected = false;
+    SocketAddress sa = new InetSocketAddress("localhost", port);
     try {
       s.connect(sa, 10000);
+      connected = true;
     } catch (IOException e) {
-      working = false;
+      LOG.error("Can't open connection to " + sa, e);
     }
+    return connected;
+  }
 
-    if (working == null) {
-      working = s.isConnected();
-      try {
-        s.close();
-      } catch (IOException e) {
-        logger.error("Can't close connection to localhost:" + py4jPort, e);
-      }
+  private void tryToClose(Socket s) {
+    try {
+      s.close();
+    } catch (IOException e) {
+      LOG.error("Can't close connection to " + s.getInetAddress(), e);
     }
-    return working;
   }
 
-
-
   private String answerFromPythonMock(InvocationOnMock invocationOnMock) {
     Object[] inputs = invocationOnMock.getArguments();
     String cmdToExecute = (String) inputs[0];


Mime
View raw message