Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ZOOKEEPER-4232: InvalidSnapshotTest corrupts its own test data #115

Merged
merged 1 commit into from
Nov 2, 2022

Conversation

anuragmadnawat1
Copy link
Owner

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

`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
@anuragmadnawat1 anuragmadnawat1 merged commit 0290017 into master Nov 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants