From 83237ab1a3d71ce5a66b5de3caae8c110f01b794 Mon Sep 17 00:00:00 2001 From: Mateusz Pietryga Date: Thu, 23 Sep 2021 00:22:31 +0200 Subject: [PATCH 1/2] Remove the unimplemented support for reflection-based errorOccurred method in SerialPortEventListener --- src/main/java/jssc/SerialPort.java | 28 ---------------------------- 1 file changed, 28 deletions(-) diff --git a/src/main/java/jssc/SerialPort.java b/src/main/java/jssc/SerialPort.java index 14f9acc9e..86b92c806 100644 --- a/src/main/java/jssc/SerialPort.java +++ b/src/main/java/jssc/SerialPort.java @@ -25,7 +25,6 @@ package jssc; import java.io.UnsupportedEncodingException; -import java.lang.reflect.Method; /** * @@ -42,10 +41,6 @@ public class SerialPort { private boolean maskAssigned = false; private boolean eventListenerAdded = false; - //since 2.2.0 -> - private volatile Method methodErrorOccurred = null; - //<- since 2.2.0 - /** Baud rate 110 symbols/second **/ public static final int BAUDRATE_110 = 110; /** Baud rate 300 symbols/second **/ @@ -1094,19 +1089,6 @@ private synchronized void addEventListener(SerialPortEventListener listener, int eventListener = listener; eventThread = getNewEventThread(); eventThread.setName("EventThread " + portName); - //since 2.2.0 -> - try { - Method method = eventListener.getClass().getMethod("errorOccurred", new Class[]{SerialPortException.class}); - method.setAccessible(true); - methodErrorOccurred = method; - } - catch (SecurityException ex) { - //Do nothing - } - catch (NoSuchMethodException ex) { - //Do nothing - } - //<- since 2.2.0 eventThread.start(); eventListenerAdded = true; } @@ -1154,7 +1136,6 @@ public synchronized boolean removeEventListener() throws SerialPortException { } } } - methodErrorOccurred = null; eventListenerAdded = false; return true; } @@ -1192,15 +1173,6 @@ public void run() { for(int[] event : eventArray){ if(event[0] > 0 && !threadTerminated){ eventListener.serialEvent(new SerialPortEvent(SerialPort.this, event[0], event[1])); - //FIXME - /*if(methodErrorOccurred != null){ - try { - methodErrorOccurred.invoke(eventListener, new Object[]{new SerialPortException(SerialPort.this, "method", "exception")}); - } - catch (Exception ex) { - System.out.println(ex); - } - }*/ } } } From eee44ef4f7214c6680c159dc003c4d96e0775c7f Mon Sep 17 00:00:00 2001 From: Mateusz Pietryga Date: Fri, 24 Sep 2021 21:11:45 +0200 Subject: [PATCH 2/2] Add Mockito based unit tests for SerialPort --- pom.xml | 12 ++ .../java/jssc/NativeMethodInvocationTest.java | 188 ++++++++++++++++++ 2 files changed, 200 insertions(+) create mode 100644 src/test/java/jssc/NativeMethodInvocationTest.java diff --git a/pom.xml b/pom.xml index 73edc2ed0..7bab623c9 100644 --- a/pom.xml +++ b/pom.xml @@ -88,6 +88,18 @@ ${dependency.junit.version} test + + org.powermock + powermock-module-junit4 + 2.0.9 + test + + + org.powermock + powermock-api-mockito2 + 2.0.9 + test + ch.qos.logback logback-classic diff --git a/src/test/java/jssc/NativeMethodInvocationTest.java b/src/test/java/jssc/NativeMethodInvocationTest.java new file mode 100644 index 000000000..801699772 --- /dev/null +++ b/src/test/java/jssc/NativeMethodInvocationTest.java @@ -0,0 +1,188 @@ +package jssc; + +import junit.framework.TestCase; +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.powermock.api.mockito.PowerMockito; +import org.powermock.core.classloader.annotations.PrepareForTest; +import org.powermock.modules.junit4.PowerMockRunner; +import org.powermock.reflect.Whitebox; + +import static org.mockito.Mockito.*; + +/** + * Tests if Java logic around native invocations does not prevent + * critical calls from happening when opening and closing ports + */ +@RunWith(PowerMockRunner.class) +@PrepareForTest(fullyQualifiedNames = "jssc.*") +public class NativeMethodInvocationTest extends TestCase { + + @Mock(name = "serialInterface") + private SerialNativeInterface serialInterface; + + private final long mockHandle = 0xDeadDeefL; + + @Test + public void nativeMethodIsNotCalledWhenAttemptingToOpenAlreadyOpenPort() { + // given + SerialPort serialPort = newSerialPort(); + + // when + try{ + serialPort.openPort(); + serialPort.openPort(); + Assert.fail("Expected to throw a SerialPortException"); + } catch (SerialPortException expected) { + // TODO: Is it really expected or should this method return false as javadoc states? + } + + // then + verify(serialInterface, times(1)).openPort(anyString(), anyBoolean()); + } + + @Test + public void nativeMethodIsNotCalledWhenAttemptingToClosePortThatIsNotYetOpen() { + // given + SerialPort serialPort = newSerialPort(); + + // when + try{ + serialPort.closePort(); + Assert.fail("Expected to throw a SerialPortException"); + } catch (SerialPortException expected) { + // TODO: Is it really expected or should this method return false as javadoc states? + } + + // then + verify(serialInterface, never()).setEventsMask(anyLong(), anyInt()); + verify(serialInterface, never()).closePort(anyLong()); + } + + @Test + public void nativeMethodIsNotCalledWhenAttemptingCloseAlreadyClosedPort() { + // given + SerialPort serialPort = newSerialPort(); + + // when + try{ + serialPort.openPort(); + serialPort.closePort(); + serialPort.closePort(); + Assert.fail("Expected to throw a SerialPortException"); + } catch (SerialPortException expected) { + // TODO: Is it really expected or should this method return false as javadoc states? + } + + // then + verify(serialInterface, times(1)).openPort(anyString(), anyBoolean()); + verify(serialInterface, times(1)).closePort(anyLong()); + } + + @Test + public void nativeMethodIsCalledWhenClosingOpenAndReopeningClosedPort() throws SerialPortException { + // given + SerialPort serialPort = newSerialPort(); + + // when + serialPort.openPort(); + serialPort.closePort(); + serialPort.openPort(); + serialPort.closePort(); + + // then + verify(serialInterface, times(2)).openPort(anyString(), anyBoolean()); + verify(serialInterface, times(2)).closePort(mockHandle); + } + + @Test + public void nativeMethodIsCalledWhenClosingOpenPortWithEventListenerOnPosix() throws SerialPortException { + // given + setOs(SerialNativeInterface.OS_LINUX); + SerialPort serialPort = newSerialPort(); + when(serialInterface.setEventsMask(anyLong(), anyInt())).thenReturn(true); + + // when + serialPort.openPort(); + serialPort.addEventListener(someListener(), SerialPort.MASK_RXCHAR); + serialPort.closePort(); + + // then + verify(serialInterface, times(1)).openPort(anyString(), anyBoolean()); + verify(serialInterface, never()).setEventsMask(anyLong(), anyInt()); + verify(serialInterface, times(1)).closePort(mockHandle); + } + + @Test + public void nativeMethodIsCalledWhenClosingOpenPortWithEventListenerOnWindows() throws SerialPortException { + // given + setOs(SerialNativeInterface.OS_WINDOWS); + SerialPort serialPort = newSerialPort(); + when(serialInterface.setEventsMask(anyLong(), anyInt())).thenReturn(true); + + // when + serialPort.openPort(); + serialPort.addEventListener(someListener(), SerialPort.MASK_RXCHAR); + serialPort.closePort(); + + // then + verify(serialInterface, times(1)).openPort(anyString(), anyBoolean()); + verify(serialInterface, times(1)).setEventsMask(mockHandle, SerialPort.MASK_RXCHAR); + verify(serialInterface, times(1)).setEventsMask(mockHandle, 0); + verify(serialInterface, times(1)).closePort(mockHandle); + } + + /** + * Simulates conditions as described in issue #107: user attempts to close the serial port that has been removed + * from the system (e.g. unplugged usb serial adapter) without notifying the java code about it. + */ + @Test + public void nativeMethodIsCalledWhenClosingOpenPortWithEventListenerIfSetMaskFails() { + // given + setOs(SerialNativeInterface.OS_WINDOWS); + SerialPort serialPort = newSerialPort(); + when(serialInterface.setEventsMask(anyLong(), anyInt())).thenReturn(true); + + // when + try { + serialPort.openPort(); + serialPort.addEventListener(someListener()); + when(serialInterface.setEventsMask(anyLong(), anyInt())).thenReturn(false); + serialPort.closePort(); + } catch (SerialPortException expected) { + // TODO: Is it really expected or should this method return false as javadoc states? + } + + // then + verify(serialInterface, times(1)).openPort(anyString(), anyBoolean()); + verify(serialInterface, times(1)).setEventsMask(mockHandle, SerialPort.MASK_RXCHAR); + verify(serialInterface, times(1)).setEventsMask(mockHandle, 0); + verify(serialInterface, times(1)).closePort(mockHandle); + } + + private void setOs(int os) { + PowerMockito.mockStatic(SerialNativeInterface.class); + when(SerialNativeInterface.getOsType()).thenReturn(os); + } + + private SerialPort newSerialPort() { + String portName = "dummy"; + SerialPort serialPort = new SerialPort(portName); + Whitebox.setInternalState(serialPort, "serialInterface", serialInterface); + when(serialInterface.openPort(anyString(), anyBoolean())).thenReturn(mockHandle); + when(serialInterface.closePort(anyLong())).thenReturn(true); + when(serialInterface.waitEvents(anyLong())).thenReturn(new int[][]{}); + return serialPort; + } + + private SerialPortEventListener someListener() { + return new SerialPortEventListener() { + @Override + public void serialEvent(SerialPortEvent serialPortEvent) { + + } + }; + } +}