Skip to content

Commit

Permalink
ZOOKEEPER-4232: InvalidSnapshotTest corrupts its own test data
Browse files Browse the repository at this point in the history
`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 <[email protected]>

Reviewers: Mohammad Arshad <[email protected]>

Closes apache#1622 from ztzg/ZOOKEEPER-4232-invalid-snapshot-is-invalid and squashes the following commits:

8b48eea [Damien Diederen] ZOOKEEPER-4232: Ensure that ZOOKEEPER-367 test data fails to parse
9250756 [Damien Diederen] ZOOKEEPER-4232: Run InvalidSnapshotTest on a copy of test data
dcf5604 [Damien Diederen] ZOOKEEPER-4232: Restore test data for ZOOKEEPER-367
  • Loading branch information
ztzg authored and anurag-harness committed Nov 2, 2022
1 parent 51b9d57 commit a912ac1
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@

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;
Expand Down Expand Up @@ -61,13 +64,36 @@ public void testSnapshotFormatterWithNull() throws Exception {
SnapshotFormatter.main(args);
}

/**
* 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]);
Expand Down
Binary file not shown.

0 comments on commit a912ac1

Please sign in to comment.