Mailing-List: contact issues-help@flink.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@flink.apache.org
From: tillrohrmann <git@git.apache.org>
To: issues@flink.incubator.apache.org
Reply-To: issues@flink.incubator.apache.org
References: <git-pr-4234-flink@git.apache.org>
In-Reply-To: <git-pr-4234-flink@git.apache.org>
Subject: [GitHub] flink pull request #4234: [FLINK-7053][blob] improve code quality in some te...
Content-Type: text/plain
Message-Id: <20170804143534.EB1A1E10F8@git1-us-west.apache.org>
Date: Fri,  4 Aug 2017 14:35:34 +0000 (UTC)
archived-at: Fri, 04 Aug 2017 14:35:40 -0000

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

    https://github.com/apache/flink/pull/4234#discussion_r131402370
  
    --- Diff: flink-runtime/src/test/java/org/apache/flink/runtime/blob/BlobClientSslTest.java ---
    @@ -107,195 +96,89 @@ public static void stopServers() throws IOException {
     		if (BLOB_SSL_SERVER != null) {
     			BLOB_SSL_SERVER.close();
     		}
    +	}
     
    -		if (BLOB_SERVER != null) {
    -			BLOB_SERVER.close();
    -		}
    +	protected Configuration getBlobClientConfig() {
    +		return sslClientConfig;
    +	}
    +
    +	protected BlobServer getBlobServer() {
    +		return BLOB_SSL_SERVER;
     	}
     
     	/**
    -	 * Prepares a test file for the unit tests, i.e. the methods fills the file with a particular byte patterns and
    -	 * computes the file's BLOB key.
    -	 *
    -	 * @param file
    -	 *        the file to prepare for the unit tests
    -	 * @return the BLOB key of the prepared file
    -	 * @throws IOException
    -	 *         thrown if an I/O error occurs while writing to the test file
    +	 * Verify ssl client to ssl server upload
     	 */
    -	private static BlobKey prepareTestFile(File file) throws IOException {
    -
    -		MessageDigest md = BlobUtils.createMessageDigest();
    -
    -		final byte[] buf = new byte[TEST_BUFFER_SIZE];
    -		for (int i = 0; i < buf.length; ++i) {
    -			buf[i] = (byte) (i % 128);
    -		}
    -
    -		FileOutputStream fos = null;
    -		try {
    -			fos = new FileOutputStream(file);
    -
    -			for (int i = 0; i < 20; ++i) {
    -				fos.write(buf);
    -				md.update(buf);
    -			}
    -
    -		} finally {
    -			if (fos != null) {
    -				fos.close();
    -			}
    -		}
    -
    -		return new BlobKey(md.digest());
    +	@Test
    +	public void testUploadJarFilesHelper() throws Exception {
    +		uploadJarFile(BLOB_SSL_SERVER, sslClientConfig);
     	}
     
     	/**
    -	 * Validates the result of a GET operation by comparing the data from the retrieved input stream to the content of
    -	 * the specified file.
    -	 *
    -	 * @param inputStream
    -	 *        the input stream returned from the GET operation
    -	 * @param file
    -	 *        the file to compare the input stream's data to
    -	 * @throws IOException
    -	 *         thrown if an I/O error occurs while reading the input stream or the file
    +	 * Verify ssl client to non-ssl server failure
     	 */
    -	private static void validateGet(final InputStream inputStream, final File file) throws IOException {
    -
    -		InputStream inputStream2 = null;
    -		try {
    -
    -			inputStream2 = new FileInputStream(file);
    -
    -			while (true) {
    -
    -				final int r1 = inputStream.read();
    -				final int r2 = inputStream2.read();
    -
    -				assertEquals(r2, r1);
    -
    -				if (r1 < 0) {
    -					break;
    -				}
    -			}
    -
    -		} finally {
    -			if (inputStream2 != null) {
    -				inputStream2.close();
    -			}
    -		}
    -
    +	@Test(expected = IOException.class)
    +	public void testSSLClientFailure() throws Exception {
    +		// SSL client connected to non-ssl server
    +		uploadJarFile(BLOB_SERVER, sslClientConfig);
     	}
     
     	/**
    -	 * Tests the PUT/GET operations for content-addressable streams.
    +	 * Verify ssl client to non-ssl server failure
     	 */
    -	@Test
    -	public void testContentAddressableStream() {
    -
    -		BlobClient client = null;
    -		InputStream is = null;
    -
    -		try {
    -			File testFile = File.createTempFile("testfile", ".dat");
    -			testFile.deleteOnExit();
    -
    -			BlobKey origKey = prepareTestFile(testFile);
    -
    -			InetSocketAddress serverAddress = new InetSocketAddress("localhost", BLOB_SSL_SERVER.getPort());
    -			client = new BlobClient(serverAddress, sslClientConfig);
    -
    -			// Store the data
    -			is = new FileInputStream(testFile);
    -			BlobKey receivedKey = client.put(is);
    -			assertEquals(origKey, receivedKey);
    -
    -			is.close();
    -			is = null;
    -
    -			// Retrieve the data
    -			is = client.get(receivedKey);
    -			validateGet(is, testFile);
    -		}
    -		catch (Exception e) {
    -			e.printStackTrace();
    -			fail(e.getMessage());
    -		}
    -		finally {
    -			if (is != null) {
    -				try {
    -					is.close();
    -				} catch (Throwable t) {}
    -			}
    -			if (client != null) {
    -				try {
    -					client.close();
    -				} catch (Throwable t) {}
    -			}
    -		}
    +	@Test(expected = IOException.class)
    +	public void testSSLClientFailure2() throws Exception {
    +		// SSL client connected to non-ssl server
    +		uploadJarFile(BLOB_NON_SSL_SERVER, sslClientConfig);
     	}
     
     	/**
    -	 * Tests the static {@link BlobClient#uploadJarFiles(InetSocketAddress, Configuration, List)} helper.
    +	 * Verify non-ssl client to ssl server failure
     	 */
    -	private void uploadJarFile(BlobServer blobServer, Configuration blobClientConfig) throws Exception {
    -		final File testFile = File.createTempFile("testfile", ".dat");
    -		testFile.deleteOnExit();
    -		prepareTestFile(testFile);
    -
    -		InetSocketAddress serverAddress = new InetSocketAddress("localhost", blobServer.getPort());
    -
    -		List<BlobKey> blobKeys = BlobClient.uploadJarFiles(serverAddress, blobClientConfig,
    -			Collections.singletonList(new Path(testFile.toURI())));
    -
    -		assertEquals(1, blobKeys.size());
    +	@Test(expected = IOException.class)
    +	public void testSSLServerFailure() throws Exception {
    +		// Non-SSL client connected to ssl server
    +		uploadJarFile(BLOB_SSL_SERVER, clientConfig);
    +	}
     
    -		try (BlobClient blobClient = new BlobClient(serverAddress, blobClientConfig)) {
    -			InputStream is = blobClient.get(blobKeys.get(0));
    -			validateGet(is, testFile);
    -		}
    +	/**
    +	 * Verify non-ssl client to ssl server failure
    +	 */
    +	@Test(expected = IOException.class)
    +	public void testSSLServerFailure2() throws Exception {
    +		// Non-SSL client connected to ssl server
    +		uploadJarFile(BLOB_SSL_SERVER, nonSslClientConfig);
     	}
     
     	/**
    -	 * Verify ssl client to ssl server upload
    +	 * Verify non-ssl connection sanity
     	 */
     	@Test
    -	public void testUploadJarFilesHelper() throws Exception {
    -		uploadJarFile(BLOB_SSL_SERVER, sslClientConfig);
    +	public void testNonSSLConnection() throws Exception {
    +		uploadJarFile(BLOB_SERVER, clientConfig);
     	}
     
     	/**
    -	 * Verify ssl client to non-ssl server failure
    +	 * Verify non-ssl connection sanity
     	 */
     	@Test
    -	public void testSSLClientFailure() throws Exception {
    -		try {
    -			uploadJarFile(BLOB_SERVER, sslClientConfig);
    -			fail("SSL client connected to non-ssl server");
    -		} catch (Exception e) {
    -			// Exception expected
    -		}
    +	public void testNonSSLConnection2() throws Exception {
    +		uploadJarFile(BLOB_SERVER, nonSslClientConfig);
    --- End diff --
    
    Why are we testing BLOB_SERVER here? Isn't this part of the `BlobClientTest`?


---
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.
---