I thought it will be an easy change, but some tests failed, as they relied on the private cloning. I was able to fix all of them except TestSnapshotDeletionPolicy.testRollbackToOldSnapshot.
I don't know if the test was always buggy, or SDP is buggy, but here's what happens if I don't clone IWC (which I put a nocommit on):
- First IW instance is created with the non-cloned SDP, the snapshots (IndexCommit instances) match the ones in the SDP instance. That is, assertSame passes.
- Second time we create an IW instance, not cloning IWC, SDP.onInit receives new IndexCommit instances, and swaps them in its map, thereby changing the SDP.commits instances vs the test's snapshots instances, which causes assertSame to fail.
This is the result of SDP.onInit which despite already having IC instances, swaps them with the new ones given by IW. I think this is correct, since that's a new IW and we should use the new IndexCommit references. In that case, the test should be relaxed to use assertEquals and not assertSame? I've put a nocommit next to calling assertSnapshotExists – changed to not check "same" IC, and the test passes without cloning ... what do you think?