From b8eec64a375b268239269fe93aaca1846ce811ea Mon Sep 17 00:00:00 2001 From: Jeroen van Erp Date: Wed, 17 Jun 2015 16:04:01 +0200 Subject: [PATCH] Added tests and categories --- build.gradle | 7 +++ .../net/schmizz/sshj/transport/Reader.java | 12 +++-- .../channel/ChannelCloseEofTest.java | 2 +- .../KeepAliveThreadTerminationTest.java | 54 +++++++++++++++++++ .../hierynomus/sshj/sftp/RemoteFileTest.java | 3 +- .../hierynomus/sshj/sftp/SFTPClientTest.java | 39 ++++++++++++++ .../sshj/test/KnownFailingTests.java | 9 ++++ .../com/hierynomus/sshj/test/SlowTests.java | 4 ++ .../sshj/{ => test}/SshFixture.java | 6 +-- .../hierynomus/sshj/test/util/FileUtil.java | 20 +++++++ .../sshj/transport/DisconnectionTest.java | 2 +- .../hierynomus/sshj/userauth/GssApiTest.java | 2 +- 12 files changed, 147 insertions(+), 13 deletions(-) create mode 100644 src/test/java/com/hierynomus/sshj/keepalive/KeepAliveThreadTerminationTest.java create mode 100644 src/test/java/com/hierynomus/sshj/sftp/SFTPClientTest.java create mode 100644 src/test/java/com/hierynomus/sshj/test/KnownFailingTests.java create mode 100644 src/test/java/com/hierynomus/sshj/test/SlowTests.java rename src/test/java/com/hierynomus/sshj/{ => test}/SshFixture.java (97%) create mode 100644 src/test/java/com/hierynomus/sshj/test/util/FileUtil.java diff --git a/build.gradle b/build.gradle index 73bd1097..26bce92b 100644 --- a/build.gradle +++ b/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" diff --git a/src/main/java/net/schmizz/sshj/transport/Reader.java b/src/main/java/net/schmizz/sshj/transport/Reader.java index 23f0ff9f..8db44636 100644 --- a/src/main/java/net/schmizz/sshj/transport/Reader.java +++ b/src/main/java/net/schmizz/sshj/transport/Reader.java @@ -48,23 +48,25 @@ 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"); diff --git a/src/test/java/com/hierynomus/sshj/connection/channel/ChannelCloseEofTest.java b/src/test/java/com/hierynomus/sshj/connection/channel/ChannelCloseEofTest.java index fcd17801..06861885 100644 --- a/src/test/java/com/hierynomus/sshj/connection/channel/ChannelCloseEofTest.java +++ b/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; diff --git a/src/test/java/com/hierynomus/sshj/keepalive/KeepAliveThreadTerminationTest.java b/src/test/java/com/hierynomus/sshj/keepalive/KeepAliveThreadTerminationTest.java new file mode 100644 index 00000000..57fbcf45 --- /dev/null +++ b/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"); + } + } + } +} diff --git a/src/test/java/com/hierynomus/sshj/sftp/RemoteFileTest.java b/src/test/java/com/hierynomus/sshj/sftp/RemoteFileTest.java index fadb0355..68e158ff 100644 --- a/src/test/java/com/hierynomus/sshj/sftp/RemoteFileTest.java +++ b/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; diff --git a/src/test/java/com/hierynomus/sshj/sftp/SFTPClientTest.java b/src/test/java/com/hierynomus/sshj/sftp/SFTPClientTest.java new file mode 100644 index 00000000..99abeff2 --- /dev/null +++ b/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(); + } + } +} diff --git a/src/test/java/com/hierynomus/sshj/test/KnownFailingTests.java b/src/test/java/com/hierynomus/sshj/test/KnownFailingTests.java new file mode 100644 index 00000000..3423fd80 --- /dev/null +++ b/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 { +} diff --git a/src/test/java/com/hierynomus/sshj/test/SlowTests.java b/src/test/java/com/hierynomus/sshj/test/SlowTests.java new file mode 100644 index 00000000..37a4f004 --- /dev/null +++ b/src/test/java/com/hierynomus/sshj/test/SlowTests.java @@ -0,0 +1,4 @@ +package com.hierynomus.sshj.test; + +public interface SlowTests { +} diff --git a/src/test/java/com/hierynomus/sshj/SshFixture.java b/src/test/java/com/hierynomus/sshj/test/SshFixture.java similarity index 97% rename from src/test/java/com/hierynomus/sshj/SshFixture.java rename to src/test/java/com/hierynomus/sshj/test/SshFixture.java index 417961a5..3ec5f0d6 100644 --- a/src/test/java/com/hierynomus/sshj/SshFixture.java +++ b/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); } } } diff --git a/src/test/java/com/hierynomus/sshj/test/util/FileUtil.java b/src/test/java/com/hierynomus/sshj/test/util/FileUtil.java new file mode 100644 index 00000000..85f67366 --- /dev/null +++ b/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); + } + } +} diff --git a/src/test/java/com/hierynomus/sshj/transport/DisconnectionTest.java b/src/test/java/com/hierynomus/sshj/transport/DisconnectionTest.java index fb033455..a5da41be 100644 --- a/src/test/java/com/hierynomus/sshj/transport/DisconnectionTest.java +++ b/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; diff --git a/src/test/java/com/hierynomus/sshj/userauth/GssApiTest.java b/src/test/java/com/hierynomus/sshj/userauth/GssApiTest.java index ab96bd76..c4c74c3e 100644 --- a/src/test/java/com/hierynomus/sshj/userauth/GssApiTest.java +++ b/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;