Return-Path: 
 <dev-return-92075-apmail-cloudstack-dev-archive=cloudstack.apache.org@cloudstack.apache.org>
X-Original-To: apmail-cloudstack-dev-archive@www.apache.org
Delivered-To: apmail-cloudstack-dev-archive@www.apache.org
Received: from mail.apache.org (hermes.apache.org [140.211.11.3])
	by minotaur.apache.org (Postfix) with SMTP id F0CC319476
	for <apmail-cloudstack-dev-archive@www.apache.org>;
 Fri, 15 Apr 2016 17:49:26 +0000 (UTC)
Received: (qmail 32299 invoked by uid 500); 15 Apr 2016 17:49:26 -0000
Delivered-To: apmail-cloudstack-dev-archive@cloudstack.apache.org
Received: (qmail 32253 invoked by uid 500); 15 Apr 2016 17:49:26 -0000
Mailing-List: contact dev-help@cloudstack.apache.org; run by ezmlm
Precedence: bulk
List-Help: <mailto:dev-help@cloudstack.apache.org>
List-Unsubscribe: <mailto:dev-unsubscribe@cloudstack.apache.org>
List-Post: <mailto:dev@cloudstack.apache.org>
List-Id: <dev.cloudstack.apache.org>
Reply-To: dev@cloudstack.apache.org
Delivered-To: mailing list dev@cloudstack.apache.org
Received: (qmail 31609 invoked by uid 99); 15 Apr 2016 17:49:25 -0000
Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org)
 (140.211.11.23)
    by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 15 Apr 2016 17:49:25 +0000
Received: by git1-us-west.apache.org (ASF Mail Server at
 git1-us-west.apache.org, from userid 33)
	id C644BDFFD9; Fri, 15 Apr 2016 17:49:25 +0000 (UTC)
From: rafaelweingartner <git@git.apache.org>
To: dev@cloudstack.apache.org
Reply-To: dev@cloudstack.apache.org
References: <git-pr-1493-cloudstack@git.apache.org>
In-Reply-To: <git-pr-1493-cloudstack@git.apache.org>
Subject: [GitHub] cloudstack pull request: CLOUDSTACK-9348: Use non-blocking
 SSL han...
Content-Type: text/plain
Message-Id: <20160415174925.C644BDFFD9@git1-us-west.apache.org>
Date: Fri, 15 Apr 2016 17:49:25 +0000 (UTC)

Github user rafaelweingartner commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1493#discussion_r59913073
  
    --- Diff: utils/src/test/java/com/cloud/utils/testcase/NioTest.java ---
    @@ -19,146 +19,198 @@
     
     package com.cloud.utils.testcase;
     
    -import java.nio.channels.ClosedChannelException;
    -import java.util.Random;
    -
    -import junit.framework.TestCase;
    -
    -import org.apache.log4j.Logger;
    -import org.junit.Assert;
    -
    +import com.cloud.utils.concurrency.NamedThreadFactory;
     import com.cloud.utils.exception.NioConnectionException;
     import com.cloud.utils.nio.HandlerFactory;
     import com.cloud.utils.nio.Link;
     import com.cloud.utils.nio.NioClient;
     import com.cloud.utils.nio.NioServer;
     import com.cloud.utils.nio.Task;
     import com.cloud.utils.nio.Task.Type;
    +import org.apache.log4j.Logger;
    +import org.junit.After;
    +import org.junit.Assert;
    +import org.junit.Before;
    +import org.junit.Test;
     
    -/**
    - *
    - *
    - *
    - *
    - */
    +import java.io.IOException;
    +import java.net.InetSocketAddress;
    +import java.nio.channels.ClosedChannelException;
    +import java.nio.channels.Selector;
    +import java.nio.channels.SocketChannel;
    +import java.util.ArrayList;
    +import java.util.List;
    +import java.util.Random;
    +import java.util.concurrent.ExecutorService;
    +import java.util.concurrent.Executors;
    +
    +public class NioTest {
     
    -public class NioTest extends TestCase {
    +    private static final Logger LOGGER = Logger.getLogger(NioTest.class);
     
    -    private static final Logger s_logger = Logger.getLogger(NioTest.class);
    +    final private int totalTestCount = 10;
    +    private int completedTestCount = 0;
     
    -    private NioServer _server;
    -    private NioClient _client;
    +    private NioServer server;
    +    private List<NioClient> clients = new ArrayList<>();
    +    private List<NioClient> maliciousClients = new ArrayList<>();
     
    -    private Link _clientLink;
    +    private ExecutorService clientExecutor = Executors.newFixedThreadPool(totalTestCount, new NamedThreadFactory("NioClientHandler"));;
    +    private ExecutorService maliciousExecutor = Executors.newFixedThreadPool(5*totalTestCount, new NamedThreadFactory("MaliciousNioClientHandler"));;
     
    -    private int _testCount;
    -    private int _completedCount;
    +    private Random randomGenerator = new Random();
    +    private byte[] testBytes;
     
         private boolean isTestsDone() {
             boolean result;
             synchronized (this) {
    -            result = _testCount == _completedCount;
    +            result = totalTestCount == completedTestCount;
             }
             return result;
         }
     
    -    private void getOneMoreTest() {
    -        synchronized (this) {
    -            _testCount++;
    -        }
    -    }
    -
         private void oneMoreTestDone() {
             synchronized (this) {
    -            _completedCount++;
    +            completedTestCount++;
             }
         }
     
    -    @Override
    +    @Before
         public void setUp() {
    -        s_logger.info("Test");
    +        LOGGER.info("Setting up Benchmark Test");
     
    -        _testCount = 0;
    -        _completedCount = 0;
    -
    -        _server = new NioServer("NioTestServer", 7777, 5, new NioTestServer());
    -        try {
    -            _server.start();
    -        } catch (final NioConnectionException e) {
    -            fail(e.getMessage());
    -        }
    +        completedTestCount = 0;
    +        testBytes = new byte[1000000];
    +        randomGenerator.nextBytes(testBytes);
     
    -        _client = new NioClient("NioTestServer", "127.0.0.1", 7777, 5, new NioTestClient());
    +        // Server configured with one worker
    +        server = new NioServer("NioTestServer", 0, 1, new NioTestServer());
             try {
    -            _client.start();
    +            server.start();
             } catch (final NioConnectionException e) {
    -            fail(e.getMessage());
    +            Assert.fail(e.getMessage());
             }
     
    -        while (_clientLink == null) {
    -            try {
    -                s_logger.debug("Link is not up! Waiting ...");
    -                Thread.sleep(1000);
    -            } catch (final InterruptedException e) {
    -                // TODO Auto-generated catch block
    -                e.printStackTrace();
    +        // 5 malicious clients per valid client
    +        for (int i = 0; i < totalTestCount; i++) {
    +            for (int j = 0; j < 5; j++) {
    +                final NioClient maliciousClient = new NioMaliciousClient("NioMaliciousTestClient-" + i, "127.0.0.1", server.getPort(), 1, new NioMaliciousTestClient());
    +                maliciousClients.add(maliciousClient);
    +                maliciousExecutor.submit(new ThreadedNioClient(maliciousClient));
                 }
    +            final NioClient client = new NioClient("NioTestClient-" + i, "127.0.0.1", server.getPort(), 1, new NioTestClient());
    +            clients.add(client);
    +            clientExecutor.submit(new ThreadedNioClient(client));
             }
         }
     
    -    @Override
    +    @After
         public void tearDown() {
    +        stopClient();
    +        stopServer();
    +    }
    +
    +    protected void stopClient() {
    +        for (NioClient client : clients) {
    +            client.stop();
    +        }
    +        for (NioClient maliciousClient : maliciousClients) {
    +            maliciousClient.stop();
    +        }
    +        LOGGER.info("Clients stopped.");
    +    }
    +
    +    protected void stopServer() {
    +        server.stop();
    +        LOGGER.info("Server stopped.");
    +    }
    +
    +    @Test
    +    public void testConnection() {
    +        final long currentTime = System.currentTimeMillis();
             while (!isTestsDone()) {
    +            if (System.currentTimeMillis() - currentTime > 600000) {
    +                Assert.fail("Failed to complete test within 600s");
    +            }
                 try {
    -                s_logger.debug(_completedCount + "/" + _testCount + " tests done. Waiting for completion");
    +                LOGGER.debug(completedTestCount + "/" + totalTestCount + " tests done. Waiting for completion");
                     Thread.sleep(1000);
                 } catch (final InterruptedException e) {
    -                // TODO Auto-generated catch block
    -                e.printStackTrace();
    +                Assert.fail(e.getMessage());
                 }
             }
    -        stopClient();
    -        stopServer();
    +        LOGGER.debug(completedTestCount + "/" + totalTestCount + " tests done.");
         }
     
    -    protected void stopClient() {
    -        _client.stop();
    -        s_logger.info("Client stopped.");
    +    protected void doServerProcess(final byte[] data) {
    +        oneMoreTestDone();
    +        Assert.assertArrayEquals(testBytes, data);
    +        LOGGER.info("Verify data received by server done.");
         }
     
    -    protected void stopServer() {
    -        _server.stop();
    -        s_logger.info("Server stopped.");
    +    public byte[] getTestBytes() {
    +        return testBytes;
         }
     
    -    protected void setClientLink(final Link link) {
    -        _clientLink = link;
    +    public class ThreadedNioClient implements Runnable {
    +        final private NioClient client;
    +        ThreadedNioClient(final NioClient client) {
    +            this.client = client;
    +        }
    +
    +        @Override
    +        public void run() {
    +            try {
    +                client.start();
    +            } catch (NioConnectionException e) {
    +                Assert.fail(e.getMessage());
    +            }
    +        }
         }
     
    -    Random randomGenerator = new Random();
    +    public class NioMaliciousClient extends NioClient {
     
    -    byte[] _testBytes;
    +        public NioMaliciousClient(String name, String host, int port, int workers, HandlerFactory factory) {
    +            super(name, host, port, workers, factory);
    +        }
     
    -    public void testConnection() {
    -        _testBytes = new byte[1000000];
    -        randomGenerator.nextBytes(_testBytes);
    -        try {
    -            getOneMoreTest();
    -            _clientLink.send(_testBytes);
    -            s_logger.info("Client: Data sent");
    -            getOneMoreTest();
    -            _clientLink.send(_testBytes);
    -            s_logger.info("Client: Data sent");
    -        } catch (final ClosedChannelException e) {
    -            // TODO Auto-generated catch block
    -            e.printStackTrace();
    +        @Override
    +        protected void init() throws IOException {
    +            _selector = Selector.open();
    +            try {
    +                _clientConnection = SocketChannel.open();
    +                LOGGER.info("Connecting to " + _host + ":" + _port);
    +                final InetSocketAddress peerAddr = new InetSocketAddress(_host, _port);
    +                _clientConnection.connect(peerAddr);
    +                // Hang in there don't do anything
    +                Thread.sleep(Long.MAX_VALUE);
    --- End diff --
    
    Sleep using "Long.MAX_VALUE"? are you sure of that?
    Why that kind os sleep in a test case? Test cases should not need to rely on such things


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---