Refactored test to be reusable for other algorithm variants

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

@@ -0,0 +1,54 @@
+package com.hierynomus.sshj.test;
+
+import net.schmizz.sshj.Config;
+import net.schmizz.sshj.DefaultConfig;
+import net.schmizz.sshj.SSHClient;
+import org.apache.sshd.server.SshServer;
+import org.junit.After;
+import org.junit.Rule;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+
+public abstract class BaseAlgorithmTest {
+    private final Logger logger = LoggerFactory.getLogger(this.getClass());
+
+    @Rule
+    public SshFixture fixture = new SshFixture(false);
+
+    @After
+    public void stopServer() {
+        fixture.stopServer();
+    }
+
+    @Test
+    public void shouldVerifyAlgorithm() throws IOException {
+        attempt(100);
+    }
+
+    private void attempt(int times) throws IOException {
+        for (int i = 0; i < times; i++) {
+            logger.info("--> Attempt {}", i);
+            verify();
+        }
+    }
+
+    private void verify() throws IOException {
+        configureServer(fixture.getServer());
+        fixture.start();
+        Config config = getClientConfig(new DefaultConfig());
+        SSHClient sshClient = fixture.connectClient(fixture.setupClient(config));
+        assertThat("should be connected", sshClient.isConnected());
+        sshClient.disconnect();
+//        fixture.stopServer();
+        fixture.stopClient();
+    }
+
+    protected abstract Config getClientConfig(DefaultConfig defaultConfig);
+
+    protected abstract void configureServer(SshServer server);
+}

src/test/java/com/hierynomus/sshj/transport/kex/KeyExchangeTest.java

@@ -1,93 +1,61 @@
 package com.hierynomus.sshj.transport.kex;
 
-import com.hierynomus.sshj.test.KnownFailingTests;
+import com.hierynomus.sshj.test.BaseAlgorithmTest;
-import com.hierynomus.sshj.test.SshFixture;
+import net.schmizz.sshj.Config;
 import net.schmizz.sshj.DefaultConfig;
-import net.schmizz.sshj.SSHClient;
 import net.schmizz.sshj.common.Factory;
-import net.schmizz.sshj.transport.kex.Curve25519SHA256;
 import net.schmizz.sshj.transport.kex.DHGexSHA1;
 import net.schmizz.sshj.transport.kex.DHGexSHA256;
 import net.schmizz.sshj.transport.kex.ECDHNistP;
-import net.schmizz.sshj.transport.verification.PromiscuousVerifier;
 import org.apache.sshd.common.NamedFactory;
 import org.apache.sshd.common.kex.BuiltinDHFactories;
+import org.apache.sshd.common.kex.KeyExchange;
+import org.apache.sshd.server.SshServer;
 import org.apache.sshd.server.kex.DHGEXServer;
 import org.apache.sshd.server.kex.DHGServer;
-import org.junit.After;
 import org.junit.Ignore;
-import org.junit.Rule;
 import org.junit.Test;
-import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
-import org.slf4j.Logger;
+import org.junit.runners.Parameterized;
-import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
+import java.util.Arrays;
+import java.util.Collection;
 import java.util.Collections;
 
 import static org.hamcrest.MatcherAssert.assertThat;
 
-public class KeyExchangeTest {
+@RunWith(Parameterized.class)
-    private static final Logger logger = LoggerFactory.getLogger(KeyExchangeTest.class);
+public class KeyExchangeTest extends BaseAlgorithmTest {
 
-    @Rule
+    @Parameterized.Parameters
-    public SshFixture fixture = new SshFixture(false);
+    public static Collection<Object[]> getParameters() {
-
+        return Arrays.asList(new Object[][]{
-    @After
+                {DHGEXServer.newFactory(BuiltinDHFactories.dhgex), new DHGexSHA1.Factory()},
-    public void stopServer() {
+                {DHGEXServer.newFactory(BuiltinDHFactories.dhgex256), new DHGexSHA256.Factory()},
-        fixture.stopServer();
+                {DHGServer.newFactory(BuiltinDHFactories.ecdhp256), new ECDHNistP.Factory256()},
+                {DHGServer.newFactory(BuiltinDHFactories.ecdhp384), new ECDHNistP.Factory384()},
+                {DHGServer.newFactory(BuiltinDHFactories.ecdhp521), new ECDHNistP.Factory521()}
+                // Not supported yet by MINA {null, new Curve25519SHA256.Factory()}
+        });
     }
 
-    @Test
+    private Factory.Named<net.schmizz.sshj.transport.kex.KeyExchange> clientFactory;
-    public void shouldKexWithDiffieHellmanGroupExchangeSha1() throws IOException {
+    private NamedFactory<KeyExchange> serverFactory;
-        setupAndCheckKex(DHGEXServer.newFactory(BuiltinDHFactories.dhgex), new DHGexSHA1.Factory());
+
+    public KeyExchangeTest(NamedFactory<KeyExchange> serverFactory, Factory.Named<net.schmizz.sshj.transport.kex.KeyExchange> clientFactory) {
+        this.clientFactory = clientFactory;
+        this.serverFactory = serverFactory;
     }
 
-    @Test
+    @Override
-    public void shouldKexWithDiffieHellmanGroupExchangeSha256() throws IOException {
+    protected Config getClientConfig(DefaultConfig config) {
-        setupAndCheckKex(DHGEXServer.newFactory(BuiltinDHFactories.dhgex256), new DHGexSHA256.Factory());
-    }
-
-    @Test
-    public void shouldKexWithEllipticCurveDiffieHellmanNistP256() throws IOException {
-        attemptKex(100, DHGServer.newFactory(BuiltinDHFactories.ecdhp256), new ECDHNistP.Factory256());
-    }
-
-    @Test
-    public void shouldKexWithEllipticCurveDiffieHellmanNistP384() throws IOException {
-        attemptKex(100, DHGServer.newFactory(BuiltinDHFactories.ecdhp384), new ECDHNistP.Factory384());
-    }
-
-    @Test
-    public void shouldKexWithEllipticCurveDiffieHellmanNistP521() throws IOException {
-        attemptKex(100, DHGServer.newFactory(BuiltinDHFactories.ecdhp521), new ECDHNistP.Factory521());
-    }
-
-    @Test
-    @Ignore("Apache SSHD does (not yet) have Curve25519 support")
-    public void shouldKexWithCurve25519() throws IOException {
-        attemptKex(100, null, new Curve25519SHA256.Factory());
-    }
-
-
-    private void attemptKex(int times, NamedFactory<org.apache.sshd.common.kex.KeyExchange> serverFactory,
-                            Factory.Named<net.schmizz.sshj.transport.kex.KeyExchange> clientFactory) throws IOException {
-        for (int i = 0; i < times; i++) {
-            logger.info("--> Attempt {}", i);
-            setupAndCheckKex(serverFactory, clientFactory);
-        }
-    }
-
-    private void setupAndCheckKex(NamedFactory<org.apache.sshd.common.kex.KeyExchange> serverFactory,
-                                  Factory.Named<net.schmizz.sshj.transport.kex.KeyExchange> clientFactory) throws IOException {
-        fixture.getServer().setKeyExchangeFactories(Collections.singletonList(serverFactory));
-        fixture.start();
-        DefaultConfig config = new DefaultConfig();
         config.setKeyExchangeFactories(Collections.singletonList(clientFactory));
-        SSHClient sshClient = fixture.connectClient(fixture.setupClient(config));
+        return config;
-        assertThat("should be connected", sshClient.isConnected());
+    }
-        sshClient.disconnect();
+
-//        fixture.stopServer();
+    @Override
-        fixture.stopClient();
+    protected void configureServer(SshServer server) {
+        server.setKeyExchangeFactories(Collections.singletonList(serverFactory));
     }
 }