From issues-return-147101-archive-asf-public=cust-asf.ponee.io@flink.apache.org Wed Jan 10 07:14:28 2018 Return-Path: X-Original-To: archive-asf-public@eu.ponee.io Delivered-To: archive-asf-public@eu.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by mx-eu-01.ponee.io (Postfix) with ESMTP id 51B2618062E for ; Wed, 10 Jan 2018 07:14:28 +0100 (CET) Received: by cust-asf.ponee.io (Postfix) id 41ACB160C3F; Wed, 10 Jan 2018 06:14:28 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id 86E9D160C17 for ; Wed, 10 Jan 2018 07:14:27 +0100 (CET) Received: (qmail 80203 invoked by uid 500); 10 Jan 2018 06:14:26 -0000 Mailing-List: contact issues-help@flink.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@flink.apache.org Delivered-To: mailing list issues@flink.apache.org Received: (qmail 80194 invoked by uid 99); 10 Jan 2018 06:14:26 -0000 Received: from pnap-us-west-generic-nat.apache.org (HELO spamd2-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 10 Jan 2018 06:14:26 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd2-us-west.apache.org (ASF Mail Server at spamd2-us-west.apache.org) with ESMTP id 277841A0139 for ; Wed, 10 Jan 2018 06:14:26 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd2-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: -4.03 X-Spam-Level: X-Spam-Status: No, score=-4.03 tagged_above=-999 required=6.31 tests=[KAM_LAZY_DOMAIN_SECURITY=1, RCVD_IN_DNSWL_HI=-5, RCVD_IN_MSPIKE_H3=-0.01, RCVD_IN_MSPIKE_WL=-0.01, T_RP_MATCHES_RCVD=-0.01] autolearn=disabled Received: from mx1-lw-us.apache.org ([10.40.0.8]) by localhost (spamd2-us-west.apache.org [10.40.0.9]) (amavisd-new, port 10024) with ESMTP id NKCiEr-PyFyj for ; Wed, 10 Jan 2018 06:14:24 +0000 (UTC) Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by mx1-lw-us.apache.org (ASF Mail Server at mx1-lw-us.apache.org) with SMTP id 5FBBB5F4AE for ; Wed, 10 Jan 2018 06:14:24 +0000 (UTC) Received: (qmail 80176 invoked by uid 99); 10 Jan 2018 06:14:23 -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; Wed, 10 Jan 2018 06:14:23 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id B67FADFC2F; Wed, 10 Jan 2018 06:14:23 +0000 (UTC) From: tzulitai To: issues@flink.incubator.apache.org Reply-To: issues@flink.incubator.apache.org Message-ID: Subject: [GitHub] flink pull request #5268: [FLINK-8398] [kinesis, tests] Harden KinesisDataFe... Content-Type: text/plain Date: Wed, 10 Jan 2018 06:14:23 +0000 (UTC) GitHub user tzulitai opened a pull request: https://github.com/apache/flink/pull/5268 [FLINK-8398] [kinesis, tests] Harden KinesisDataFetcherTest unit tests ## What is the purpose of the change Prior to this PR, many of the `KinesisDataFetcherTest` unit tests relied on thread sleeps to wait until a certain operation occurs to allow the test to pass. This test behaviour is very flaky, and should be replaced with `OneShotLatch`. ## Brief change log - 94b4591: Several minor cleanups of confusing implementations / code smells in the `KinesisDataFetcherTest` and related test classes. The commit message explains what exactly was changed. - 547d19f: Remove thread sleeps in unit tests, and replace them with `OneShotLatch`. ## Verifying this change No test coverage should have been affected by this change. The existing tests in `KinesisDataFetcherTest` verifies this. ## Does this pull request potentially affect one of the following parts: - Dependencies (does it add or upgrade a dependency): (yes / *no*) - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (yes / *no*) - The serializers: (yes / *no* / don't know) - The runtime per-record code paths (performance sensitive): (yes / *no* / don't know) - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (yes / *no* / don't know) - The S3 file system connector: (yes / *no* / don't know) ## Documentation - Does this pull request introduce a new feature? (yes / *no*) - If yes, how is the feature documented? (*not applicable* / docs / JavaDocs / not documented) You can merge this pull request into a Git repository by running: $ git pull https://github.com/tzulitai/flink FLINK-8398 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/5268.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 #5268 ---- commit 94b45919afa5a3ec3ce68c45e57f7989397f9640 Author: Tzu-Li (Gordon) Tai Date: 2018-01-10T02:11:31Z [FLINK-8398] [kinesis, tests] Cleanup confusing implementations in KinesisDataFetcherTest and related classes The previous implementation of the TestableKinesisDataFetcher was confusing in various ways, causing it hard to be re-used for other tests. This commit contains the following various cleaups: - Remove confusing mocks of source context and checkpoint lock. We now allow users of the TestableKinesisDataFetcher to provide a source context, which should provide the checkpoint lock. - Remove override of emitRecordAndUpdateState(). Strictly speaking, that method should be final. It was previously overriden to allow verifying how many records were output by the fetcher. That verification would be better implemented within a mock source context. - Properly parameterize the output type for the TestableKinesisDataFetcher. - Remove use of PowerMockito in KinesisDataFetcherTest. - Use CheckedThreads to properly capture any exceptions in fetcher / consumer threads in unit tests. - Use assertEquals / assertNull instead of assertTrue where-ever appropriate. commit 547d19f9196512231661f427f3792f2e1f831339 Author: Tzu-Li (Gordon) Tai Date: 2018-01-10T05:41:49Z [FLINK-8398] [kinesis, tests] Stabilize flaky KinesisDataFetcherTests Prior to this commit, several unit tests in KinesisDataFetcherTest relied on sleeps to wait until a certain operation happens, in order for the test to pass. This commit removes those sleeps and replaces the test behaviours with OneShotLatches. ---- ---