Added tests and categories

build.gradle

@@ -22,6 +22,13 @@ configurations {
 
 test {
   include "**/*Test.*"
+  if (!project.hasProperty("allTests")) {
+    useJUnit {
+      excludeCategories 'com.hierynomus.sshj.test.SlowTests'
+      excludeCategories 'com.hierynomus.sshj.test.KnownFailingTests'
+    }
+  }
+
   afterSuite { descriptor, result ->
     if (descriptor.className != null) {
       def indicator = "\u001B[32m✓\u001b[0m"

src/main/java/net/schmizz/sshj/transport/Reader.java

@@ -48,24 +48,26 @@ public final class Reader
                 int read;
                 try {
                     read = inp.read(recvbuf, 0, needed);
-                }
+                } catch(SocketTimeoutException e) {
-                catch(SocketTimeoutException e) {
                     if (isInterrupted()) {
                         throw e;
                     }
                     continue;
                 }
-                if (read == -1)
+                if (read == -1) {
                     throw new TransportException("Broken transport; encountered EOF");
-                else
+                } else {
                     needed = decoder.received(recvbuf, read);
                 }
+            }
         } catch (Exception e) {
+            //noinspection StatementWithEmptyBody
             if (isInterrupted()) {
                 // We are meant to shut up and draw to a close if interrupted
-            } else
+            } else {
                 trans.die(e);
             }
+        }
 
         log.debug("Stopping");
     }

src/test/java/com/hierynomus/sshj/connection/channel/ChannelCloseEofTest.java

@@ -1,6 +1,6 @@
 package com.hierynomus.sshj.connection.channel;
 
-import com.hierynomus.sshj.SshFixture;
+import com.hierynomus.sshj.test.SshFixture;
 import net.schmizz.sshj.connection.channel.direct.Session;
 import org.junit.Rule;
 import org.junit.Test;

src/test/java/com/hierynomus/sshj/keepalive/KeepAliveThreadTerminationTest.java

@@ -0,0 +1,54 @@
+package com.hierynomus.sshj.keepalive;
+
+import com.hierynomus.sshj.test.KnownFailingTests;
+import com.hierynomus.sshj.test.SlowTests;
+import com.hierynomus.sshj.test.SshFixture;
+import net.schmizz.keepalive.KeepAliveProvider;
+import net.schmizz.sshj.DefaultConfig;
+import net.schmizz.sshj.SSHClient;
+import net.schmizz.sshj.userauth.UserAuthException;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.io.IOException;
+import java.lang.management.ManagementFactory;
+import java.lang.management.ThreadInfo;
+import java.lang.management.ThreadMXBean;
+
+import static org.junit.Assert.fail;
+
+public class KeepAliveThreadTerminationTest {
+
+    @Rule
+    public SshFixture fixture = new SshFixture();
+
+    @Test
+    @Category({SlowTests.class, KnownFailingTests.class})
+    public void shouldCorrectlyTerminateThreadOnDisconnect() throws IOException, InterruptedException {
+        DefaultConfig defaultConfig = new DefaultConfig();
+        defaultConfig.setKeepAliveProvider(KeepAliveProvider.KEEP_ALIVE);
+        for (int i = 0; i < 10; i++) {
+            SSHClient sshClient = fixture.setupClient(defaultConfig);
+            fixture.connectClient(sshClient);
+            sshClient.getConnection().getKeepAlive().setKeepAliveInterval(1);
+            try {
+                sshClient.authPassword("bad", "credentials");
+                fail("Should not auth.");
+            } catch (UserAuthException e) {
+                // OK
+            }
+            fixture.stopClient();
+            Thread.sleep(2000);
+        }
+
+        ThreadMXBean threadMXBean = ManagementFactory.getThreadMXBean();
+        for (long l : threadMXBean.getAllThreadIds()) {
+            ThreadInfo threadInfo = threadMXBean.getThreadInfo(l);
+            if (threadInfo.getThreadName().equals("keep-alive") && threadInfo.getThreadState() != Thread.State.TERMINATED) {
+                System.err.println("Found thread in state " + threadInfo.getThreadState());
+                throw new RuntimeException("Found alive keep-alive thread");
+            }
+        }
+    }
+}

src/test/java/com/hierynomus/sshj/sftp/RemoteFileTest.java

@@ -1,6 +1,6 @@
 package com.hierynomus.sshj.sftp;
 
-import com.hierynomus.sshj.SshFixture;
+import com.hierynomus.sshj.test.SshFixture;
 import net.schmizz.sshj.SSHClient;
 import net.schmizz.sshj.sftp.OpenMode;
 import net.schmizz.sshj.sftp.RemoteFile;
@@ -12,7 +12,6 @@ import org.junit.rules.TemporaryFolder;
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
-import java.util.Arrays;
 import java.util.EnumSet;
 import java.util.Random;
 

src/test/java/com/hierynomus/sshj/sftp/SFTPClientTest.java

@@ -0,0 +1,39 @@
+package com.hierynomus.sshj.sftp;
+
+import com.hierynomus.sshj.test.SshFixture;
+import com.hierynomus.sshj.test.util.FileUtil;
+import net.schmizz.sshj.SSHClient;
+import net.schmizz.sshj.sftp.SFTPClient;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+import java.io.File;
+import java.io.IOException;
+
+public class SFTPClientTest {
+
+    @Rule
+    public SshFixture fixture = new SshFixture();
+
+    @Rule
+    public TemporaryFolder temp = new TemporaryFolder();
+
+    @Test
+    public void shouldNotThrowExceptionOnCloseBeforeDisconnect() throws IOException {
+        SSHClient sshClient = fixture.setupConnectedDefaultClient();
+        sshClient.authPassword("test", "test");
+        SFTPClient sftpClient = sshClient.newSFTPClient();
+        File file = temp.newFile("source.txt");
+        FileUtil.writeToFile(file, "This is the source");
+        try {
+            try {
+                sftpClient.put(file.getPath(), temp.newFile("dest.txt").getPath());
+             } finally {
+                sftpClient.close();
+            }
+        } finally {
+            sshClient.disconnect();
+        }
+    }
+}

src/test/java/com/hierynomus/sshj/test/KnownFailingTests.java

@@ -0,0 +1,9 @@
+package com.hierynomus.sshj.test;
+
+/**
+ * Marker interface for JUnit Categories.
+ *
+ * This denotes that the test is known to fail, and should be fixed at some time.
+ */
+public interface KnownFailingTests {
+}

src/test/java/com/hierynomus/sshj/test/SlowTests.java

@@ -0,0 +1,4 @@
+package com.hierynomus.sshj.test;
+
+public interface SlowTests {
+}

src/test/java/com/hierynomus/sshj/test/SshFixture.java

@@ -1,4 +1,4 @@
-package com.hierynomus.sshj;
+package com.hierynomus.sshj.test;
 
 import net.schmizz.sshj.Config;
 import net.schmizz.sshj.DefaultConfig;
@@ -121,7 +121,7 @@ public class SshFixture extends ExternalResource {
             try {
                 client.disconnect();
             } catch (IOException e) {
-                // Ignore
+                throw new RuntimeException(e);
             } finally {
                 client = null;
             }
@@ -135,7 +135,7 @@ public class SshFixture extends ExternalResource {
             try {
                 server.stop();
             } catch (InterruptedException e) {
-                // ignore
+                throw new RuntimeException(e);
             }
         }
     }

src/test/java/com/hierynomus/sshj/test/util/FileUtil.java

@@ -0,0 +1,20 @@
+package com.hierynomus.sshj.test.util;
+
+import net.schmizz.sshj.common.IOUtils;
+
+import java.io.File;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.io.Writer;
+
+public class FileUtil {
+
+    public static void writeToFile(File f, String content) throws IOException {
+        FileWriter w = new FileWriter(f);
+        try {
+            w.write(content);
+        } finally {
+            IOUtils.closeQuietly(w);
+        }
+    }
+}

src/test/java/com/hierynomus/sshj/transport/DisconnectionTest.java

@@ -1,6 +1,6 @@
 package com.hierynomus.sshj.transport;
 
-import com.hierynomus.sshj.SshFixture;
+import com.hierynomus.sshj.test.SshFixture;
 import net.schmizz.sshj.SSHClient;
 import net.schmizz.sshj.common.DisconnectReason;
 import net.schmizz.sshj.transport.DisconnectListener;

src/test/java/com/hierynomus/sshj/userauth/GssApiTest.java

@@ -1,6 +1,6 @@
 package com.hierynomus.sshj.userauth;
 
-import com.hierynomus.sshj.SshFixture;
+import com.hierynomus.sshj.test.SshFixture;
 import net.schmizz.sshj.SSHClient;
 import net.schmizz.sshj.userauth.method.AuthGssApiWithMic;
 import net.schmizz.sshj.util.gss.BogusGSSManager;