[bookkeeper] branch master updated: Fix TestBKSyncLogReader

[si...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

sijie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git


The following commit(s) were added to refs/heads/master by this push:
     new 2d6c304  Fix TestBKSyncLogReader
2d6c304 is described below

commit 2d6c30472ccb43eb04b1934b4c0be30283c2864e
Author: Sijie Guo <si...@apache.org>
AuthorDate: Fri Mar 2 17:58:28 2018 -0800

    Fix TestBKSyncLogReader
    
    Descriptions of the changes in this PR:
    
    *Problem*
    
    The test case `TestBKSyncLogReader` is waiting an entry beyond a control record, where the number of control records can
    vary depend on the timing of flush()/commit(). This introduces the flakiness of this test.
    
    *Solution*
    
    This change changes to wait for readahead go beyond the last user record, which is more deterministic.
    
    Author: Sijie Guo <si...@apache.org>
    
    Reviewers: Yiming Zang <yz...@gmail.com>, Jia Zhai <None>,  Philip Su <ps...@twitter.com>
    
    This closes #1168 from sijie/fix_test_sync_log_reader
---
 .travis_scripts/before_install.sh                  | 40 ++++++++++++++++++++++
 .../apache/distributedlog/TestBKSyncLogReader.java | 23 +++++--------
 2 files changed, 48 insertions(+), 15 deletions(-)

diff --git a/.travis_scripts/before_install.sh b/.travis_scripts/before_install.sh
new file mode 100755
index 0000000..b5b141f
--- /dev/null
+++ b/.travis_scripts/before_install.sh
@@ -0,0 +1,40 @@
+#!/usr/bin/env bash
+
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+set -ev
+
+echo "MAVEN_OPTS='-Xmx3072m -XX:MaxPermSize=512m'" > ~/.mavenrc
+if [ "$TRAVIS_OS_NAME" == "osx" ]; then
+    export JAVA_HOME=$(/usr/libexec/java_home);
+fi
+echo "TRAVIS_PULL_REQUEST=${TRAVIS_PULL_REQUEST}"
+if [ "$TRAVIS_PULL_REQUEST" == "false" ]; then
+    export DLOG_MODIFIED="true"  
+    echo "Enable testing distributedlog modules since they are not pull requests."
+else
+    if [ `git diff --name-only $TRAVIS_COMMIT_RANGE | grep "^stream\/distributedlog" | wc -l` -gt 0 ]; then
+        export DLOG_MODIFIED="true"  
+        echo "Enable testing distributedlog modules if this pull request modifies files under directory 'stream/distributedlog'."
+    fi
+    if [ `git diff --name-only $TRAVIS_COMMIT_RANGE | grep "^site\/" | wc -l` -gt 0 ]; then
+        # building the website to ensure the changes don't break
+        export WEBSITE_MODIFIED="true"
+        echo "Enable building website modules if this pull request modifies files under directory 'site'."
+    fi
+fi
diff --git a/stream/distributedlog/core/src/test/java/org/apache/distributedlog/TestBKSyncLogReader.java b/stream/distributedlog/core/src/test/java/org/apache/distributedlog/TestBKSyncLogReader.java
index a5d4c16..ec99e7a 100644
--- a/stream/distributedlog/core/src/test/java/org/apache/distributedlog/TestBKSyncLogReader.java
+++ b/stream/distributedlog/core/src/test/java/org/apache/distributedlog/TestBKSyncLogReader.java
@@ -184,26 +184,19 @@ public class TestBKSyncLogReader extends TestDistributedLogBase {
 
         logger.info("Write another 10 records");
 
-        // wait until readahead move on
-        while (reader.getReadAheadReader().getNextEntryPosition().getEntryId() < 21) {
-            TimeUnit.MILLISECONDS.sleep(20);
-        }
-
-        logger.info("ReadAhead is caught up with another 10 records");
-
-        // resume reading from sync reader. so it should be able to read all 20 records
-        // and return null to claim it as caughtup
-        LogRecord record = reader.readNext(false);
-        int numReads = 0;
+        // resume reading from sync reader util it consumes 20 records
         long expectedTxId = 1L;
-        while (null != record) {
-            ++numReads;
+        for (int i = 0; i < 20; i++) {
+            LogRecord record = reader.readNext(false);
+            while (null == record) {
+                record = reader.readNext(false);
+            }
             assertEquals(expectedTxId, record.getTransactionId());
             DLMTestUtil.verifyLogRecord(record);
             ++expectedTxId;
-            record = reader.readNext(false);
         }
-        assertEquals(20, numReads);
+        // after read 20 records, it should return null
+        assertNull(reader.readNext(false));
 
         out.close();
         reader.close();

-- 
To stop receiving notification emails like this one, please contact
sijie@apache.org.