You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zookeeper.apache.org by ar...@apache.org on 2021/03/09 17:15:10 UTC

[zookeeper] branch branch-3.7 updated: ZOOKEEPER-4232: InvalidSnapshotTest corrupts its own test data

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

arshad pushed a commit to branch branch-3.7
in repository https://gitbox.apache.org/repos/asf/zookeeper.git


The following commit(s) were added to refs/heads/branch-3.7 by this push:
     new 4a19dd1  ZOOKEEPER-4232: InvalidSnapshotTest corrupts its own test data
4a19dd1 is described below

commit 4a19dd12ed50bca4ae17f1fa0ce01c551d9d4915
Author: Damien Diederen <dd...@crosstwine.com>
AuthorDate: Tue Mar 9 22:47:26 2021 +0530

    ZOOKEEPER-4232: InvalidSnapshotTest corrupts its own test data
    
    `InvalidSnapshotTest.testSnapshot` starts an instance of
    `ZooKeeperServer` on the version-controlled `resources/data/invalidsnap`
    directory, which, as a side-effect, \"fixes\" the following
    snapshot—which was broken on purpose (see ZOOKEEPER-367):
    
    `zookeeper-server/src/test/resources/data/invalidsnap/version-2/snapshot.83f`
    
    This status quo creates a number of problems:
    
    1.  It makes the test ineffective after the first run;
    2.  The file shows as modified in version control tools, which can be
        annoying;
    3.  The \"fixed\" snapshot can end up being committed by mistake,
        invalidating the test.
    
    (\#3 is not theoretical; that \"fixed\" snapshot frequently shows up in
    pull requests, and was recently merged into `master`.)
    
    Author: Damien Diederen <dd...@crosstwine.com>
    
    Reviewers: Mohammad Arshad <ar...@apache.org>
    
    Closes #1627 from ztzg/ZOOKEEPER-4232-invalid-snapshot-is-invalid-x-3.7 and squashes the following commits:
    
    c18a895fa [Damien Diederen] ZOOKEEPER-4232: Ensure that ZOOKEEPER-367 test data fails to parse
    42a5a0b6d [Damien Diederen] ZOOKEEPER-4232: Run InvalidSnapshotTest on a copy of test data
---
 .../apache/zookeeper/test/InvalidSnapshotTest.java | 28 +++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/test/InvalidSnapshotTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/test/InvalidSnapshotTest.java
index 8026705..c47a60f 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/test/InvalidSnapshotTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/test/InvalidSnapshotTest.java
@@ -20,7 +20,10 @@ package org.apache.zookeeper.test;
 
 import static org.apache.zookeeper.test.ClientBase.CONNECTION_TIMEOUT;
 import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
 import java.io.File;
+import java.io.IOException;
+import org.apache.commons.io.FileUtils;
 import org.apache.zookeeper.PortAssignment;
 import org.apache.zookeeper.ZKTestCase;
 import org.apache.zookeeper.ZooKeeper;
@@ -62,12 +65,35 @@ public class InvalidSnapshotTest extends ZKTestCase {
     }
 
     /**
+     * Verify the SnapshotFormatter fails as expected on corrupted snapshot.
+     */
+    @Test
+    public void testSnapshotFormatterWithInvalidSnap() throws Exception {
+        File snapDir = new File(testData, "invalidsnap");
+        // Broken snapshot introduced by ZOOKEEPER-367, and used to
+        // demonstrate recovery in testSnapshot below.
+        File snapfile = new File(new File(snapDir, "version-2"), "snapshot.83f");
+        String[] args = {snapfile.getCanonicalFile().toString()};
+        try {
+            SnapshotFormatter.main(args);
+            fail("Snapshot '" + snapfile + "' unexpectedly parsed without error.");
+        } catch (IOException e) {
+            assertTrue(e.getMessage().contains("Unreasonable length = 977468229"));
+        }
+    }
+
+    /**
      * test the snapshot
      * @throws Exception an exception could be expected
      */
     @Test
     public void testSnapshot() throws Exception {
-        File snapDir = new File(testData, "invalidsnap");
+        File origSnapDir = new File(testData, "invalidsnap");
+
+        // This test otherwise updates the resources directory.
+        File snapDir = ClientBase.createTmpDir();
+        FileUtils.copyDirectory(origSnapDir, snapDir);
+
         ZooKeeperServer zks = new ZooKeeperServer(snapDir, snapDir, 3000);
         SyncRequestProcessor.setSnapCount(1000);
         final int PORT = Integer.parseInt(HOSTPORT.split(":")[1]);