You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zeppelin.apache.org by bz...@apache.org on 2016/06/28 10:07:50 UTC
zeppelin git commit: ZEPPELIN-1063: fix flaky python interpreter test
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 <bz...@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 <bz...@apache.org>
Authored: Mon Jun 27 20:43:09 2016 +0900
Committer: Alexander Bezzubov <bz...@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];