You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by NicoK <gi...@git.apache.org> on 2017/06/30 15:05:01 UTC

[GitHub] flink pull request #4234: [FLINK-7053] improve code quality in some tests

GitHub user NicoK opened a pull request:

    https://github.com/apache/flink/pull/4234

    [FLINK-7053] improve code quality in some tests

    * `BlobClientTest` and `BlobClientSslTest` share a lot of common code
    * the received buffers there are currently not verified for being equal to the expected one
    * `TemporaryFolder` should be used throughout blob store tests
    
    This PR is based upon #4158 in a series of PRs to implement FLINK-6916.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/NicoK/flink flink-6916-7053

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/flink/pull/4234.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #4234
    
----
commit d54a316cfffd8243980df561fd4fcbd99934a40b
Author: Nico Kruber <ni...@data-artisans.com>
Date:   2016-12-20T15:49:57Z

    [FLINK-6008][docs] minor improvements in the BlobService docs

commit b215515fa14d3f6af218e86b67bc2c27ae9d4f4f
Author: Nico Kruber <ni...@data-artisans.com>
Date:   2016-12-20T17:27:13Z

    [FLINK-6008] refactor BlobCache#getURL() for cleaner code

commit bbcde52b3105fcf379c852b568f3893cc6052ce6
Author: Nico Kruber <ni...@data-artisans.com>
Date:   2016-12-21T15:23:29Z

    [FLINK-6008] do not fail the BlobServer if delete fails
    
    also extend the delete tests and remove one code duplication

commit dda1a12e40027724efb0e50005e5b57058a220f0
Author: Nico Kruber <ni...@data-artisans.com>
Date:   2017-01-06T17:42:58Z

    [FLINK-6008][docs] update some config options to the new, non-deprecated ones

commit e12c2348b237207a50649d515a0fbbd19f92e6a0
Author: Nico Kruber <ni...@data-artisans.com>
Date:   2017-03-09T17:14:02Z

    [FLINK-6008] use Preconditions.checkArgument in BlobClient

commit 24060e01332c6df9fd01f1dc5f321c3fda9301c1
Author: Nico Kruber <ni...@data-artisans.com>
Date:   2017-03-17T15:21:40Z

    [FLINK-6008] fix concurrent job directory creation
    
    also add according unit tests

commit 2e0d16ab8bf8a48a2d028602a3a7693fc4b76039
Author: Nico Kruber <ni...@data-artisans.com>
Date:   2017-06-14T16:01:47Z

    [FLINK-6008] do not guard a delete() call with a check for existence

commit 7ba911d7ecb4861261dff8509996be0bd64d6d27
Author: Nico Kruber <ni...@data-artisans.com>
Date:   2017-04-18T14:37:37Z

    [FLINK-6008] some comments about BlobLibraryCacheManager cleanup

commit d3f50d595f85356ae6ed0a85e1f8b8e8ac630bde
Author: Nico Kruber <ni...@data-artisans.com>
Date:   2017-04-19T13:39:03Z

    [hotfix] minor typos

commit 79b6ce35a9e246b35415a388295f9ee2fc19a82e
Author: Nico Kruber <ni...@data-artisans.com>
Date:   2017-04-19T14:10:16Z

    [FLINK-6008] further cleanup tests for BlobLibraryCacheManager

commit 23fb6ecd6c43c86d762503339c67953290236dca
Author: Nico Kruber <ni...@data-artisans.com>
Date:   2017-06-30T14:03:16Z

    [FLINK-6008] address PR comments

commit 794764ceeed6b9bbbac08662f5754b218ff86c9c
Author: Nico Kruber <ni...@data-artisans.com>
Date:   2017-06-16T08:51:04Z

    [FLINK-7052][blob] remove (unused) NAME_ADDRESSABLE mode

commit 774bafa85f242110a2ce7907c1150f8c62d73b3f
Author: Nico Kruber <ni...@data-artisans.com>
Date:   2017-06-21T15:05:57Z

    [FLINK-7052][blob] remove further unused code due to the NAME_ADDRESSABLE removal

commit 4da3b3f6269e43bf1c66621099528824cad9373f
Author: Nico Kruber <ni...@data-artisans.com>
Date:   2017-06-22T15:31:17Z

    [FLINK-7053][blob] remove code duplication in BlobClientSslTest
    
    This lets BlobClientSslTest extend BlobClientTest as most of its implementation
    came from there and was simply copied.

commit aa9cdc820f9ca1a38a19708bf45a2099e42eaf48
Author: Nico Kruber <ni...@data-artisans.com>
Date:   2017-06-23T09:40:34Z

    [FLINK-7053][blob] verify some of the buffers returned by GET

commit c9b693a46053b55b3939ff471184796f12d36a72
Author: Nico Kruber <ni...@data-artisans.com>
Date:   2017-06-23T10:04:10Z

    [FLINK-7053][blob] use TemporaryFolder for local BLOB dir in unit tests
    
    This replaces the use of some temporary directory where it is not guaranteed
    that it will be deleted after the test.

----


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

[GitHub] flink pull request #4234: [FLINK-7053][blob] improve code quality in some te...

Posted by NicoK <gi...@git.apache.org>.
Github user NicoK commented on a diff in the pull request:

    https://github.com/apache/flink/pull/4234#discussion_r131431731
  
    --- 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();
     		}
    --- End diff --
    
    good catch


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

[GitHub] flink pull request #4234: [FLINK-7053][blob] improve code quality in some te...

Posted by tillrohrmann <gi...@git.apache.org>.
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.
---

[GitHub] flink pull request #4234: [FLINK-7053][blob] improve code quality in some te...

Posted by NicoK <gi...@git.apache.org>.
Github user NicoK commented on a diff in the pull request:

    https://github.com/apache/flink/pull/4234#discussion_r131431720
  
    --- Diff: flink-runtime/src/test/java/org/apache/flink/runtime/blob/BlobCacheSuccessTest.java ---
    @@ -92,15 +100,15 @@ private void uploadFileGetTest(final Configuration config, boolean cacheWorksWit
     		BlobCache blobCache = null;
     		BlobStoreService blobStoreService = null;
     		try {
    -			final Configuration cacheConfig;
    -			if (cacheHasAccessToFs) {
    -				cacheConfig = config;
    -			} else {
    -				// just in case parameters are still read from the server,
    -				// create a separate configuration object for the cache
    -				cacheConfig = new Configuration(config);
    +			final Configuration cacheConfig = new Configuration(config);
    +			cacheConfig.setString(BlobServerOptions.STORAGE_DIRECTORY,
    +				temporaryFolder.newFolder().getAbsolutePath());
    +			if (!cacheHasAccessToFs) {
    +				// make sure the cache cannot access the HA store directly
    +				cacheConfig.setString(BlobServerOptions.STORAGE_DIRECTORY,
    +					temporaryFolder.newFolder().getAbsolutePath());
    --- End diff --
    
    seems that it wasn't used to its full extension - also the meaning of this variable was not what clear


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

[GitHub] flink issue #4234: [FLINK-7053][blob] improve code quality in some tests

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:

    https://github.com/apache/flink/pull/4234
  
    @NicoK Could you rebase this PR?


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

[GitHub] flink pull request #4234: [FLINK-7053][blob] improve code quality in some te...

Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:

    https://github.com/apache/flink/pull/4234#discussion_r131401070
  
    --- Diff: flink-runtime/src/test/java/org/apache/flink/runtime/blob/BlobCacheSuccessTest.java ---
    @@ -92,15 +100,15 @@ private void uploadFileGetTest(final Configuration config, boolean cacheWorksWit
     		BlobCache blobCache = null;
     		BlobStoreService blobStoreService = null;
     		try {
    -			final Configuration cacheConfig;
    -			if (cacheHasAccessToFs) {
    -				cacheConfig = config;
    -			} else {
    -				// just in case parameters are still read from the server,
    -				// create a separate configuration object for the cache
    -				cacheConfig = new Configuration(config);
    +			final Configuration cacheConfig = new Configuration(config);
    +			cacheConfig.setString(BlobServerOptions.STORAGE_DIRECTORY,
    +				temporaryFolder.newFolder().getAbsolutePath());
    +			if (!cacheHasAccessToFs) {
    +				// make sure the cache cannot access the HA store directly
    +				cacheConfig.setString(BlobServerOptions.STORAGE_DIRECTORY,
    +					temporaryFolder.newFolder().getAbsolutePath());
    --- End diff --
    
    isn't this redundant?


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

[GitHub] flink pull request #4234: [FLINK-7053][blob] improve code quality in some te...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/flink/pull/4234


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

[GitHub] flink issue #4234: [FLINK-7053][blob] improve code quality in some tests

Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on the issue:

    https://github.com/apache/flink/pull/4234
  
    Changes look good to me. Thanks for the work @NicoK. Merging this PR.


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

[GitHub] flink pull request #4234: [FLINK-7053][blob] improve code quality in some te...

Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:

    https://github.com/apache/flink/pull/4234#discussion_r131401420
  
    --- 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();
     		}
    --- End diff --
    
    What about shutting the non ssl blob server down?


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

[GitHub] flink pull request #4234: [FLINK-7053][blob] improve code quality in some te...

Posted by NicoK <gi...@git.apache.org>.
Github user NicoK commented on a diff in the pull request:

    https://github.com/apache/flink/pull/4234#discussion_r131431976
  
    --- 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 --
    
    the client configuration is different - although both have `BlobServerOptions.SSL_ENABLED` disabled, this one has `SecurityOptions.SSL_ENABLED` enabled


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

[GitHub] flink issue #4234: [FLINK-7053][blob] improve code quality in some tests

Posted by NicoK <gi...@git.apache.org>.
Github user NicoK commented on the issue:

    https://github.com/apache/flink/pull/4234
  
    ok, I rebased this and all following PRs for FLINK-6916


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