On Sat, Oct 5, 2024 at 11:26 AM Claude Warren <cla...@xenei.com> wrote:
> > > > WrappedBloomFilter is not a WrappedBloomFilter, > > > This is true in most useful cases for the test ` > assertEquals(bf1.getClass(), copy.getClass()); > > In a proposed KIP (Kafka Improvement Proposal) there is a Bloom filter that > is decorated with additional data (The timestamp for when it was created). > This class is called TimestampedBloomFilter and it extends > WrappedBloomFilter. > Hi Claude, We have org.apache.commons.collections4.bloomfilter.LayeredBloomFilterTest.TimestampedBloomFilter. How is the KIP different? Gary > > When TimestampedBloomFilter is passed to the WrappedBloomFilterTest classes > and the "copy" method is called, the result is not a WrappedBloomFilter but > a TimestampedBloomFilter. > > If the TimestampedBloomFilterTest extends the WrappedBloomFilterTest the > test fails. > > > > > On Sat, Oct 5, 2024 at 3:21 PM Gary Gregory <garydgreg...@gmail.com> > wrote: > > > The WrappedBloomFilterTest currently says [0][1] that a copy of a > > WrappedBloomFilter is not a WrappedBloomFilter, which does not make sense > > to me as non-SME. > > > > Should the test be adjusted? It looks like the test is written in a > > contrived way due to WrappedBloomFilter being abstract. I would make more > > sense to me to have private static WrappedBloomFilter in the test and use > > it, like this: > > > > private static class Fixture extends WrappedBloomFilter { > > > > public Fixture(BloomFilter wrapped) { > > super(wrapped); > > } > > > > @Override > > public WrappedBloomFilter copy() { > > return new Fixture(getWrapped()); > > } > > > > } > > > > @Override > > protected WrappedBloomFilter createEmptyFilter(final Shape shape) { > > return new Fixture(new > > DefaultBloomFilterTest.SparseDefaultBloomFilter(shape)); > > } > > > > @ParameterizedTest > > @ValueSource(ints = {0, 1, 34}) > > public void testCharacteristics(final int characteristics) { > > final Shape shape = getTestShape(); > > final BloomFilter inner = new > > DefaultBloomFilterTest.SparseDefaultBloomFilter(shape) { > > @Override > > public int characteristics() { > > return characteristics; > > } > > }; > > final WrappedBloomFilter underTest = new Fixture(inner); > > assertEquals(characteristics, underTest.characteristics()); > > } > > > > WDYT? > > > > Let's skip discussing the signature of the copy() method, I'll write a > > separate email later about that. > > > > TY, > > Gary > > [0] > > > > > https://github.com/apache/commons-collections/blob/730d972cdebb13dd3a896eb5b90ebc9e1f594d5b/src/test/java/org/apache/commons/collections4/bloomfilter/WrappedBloomFilterTest.java#L26-L56 > > [1] The above link in WrappedBloomFilterTest is: > > > > @Override > > protected WrappedBloomFilter createEmptyFilter(final Shape shape) { > > return new WrappedBloomFilter(new > > DefaultBloomFilterTest.SparseDefaultBloomFilter(shape)) { > > @Override > > public BloomFilter copy() { > > final BloomFilter result = new > > DefaultBloomFilterTest.SparseDefaultBloomFilter(shape); > > result.merge(getWrapped()); > > return result; > > } > > }; > > } > > > > @ParameterizedTest > > @ValueSource(ints = {0, 1, 34}) > > public void testCharacteristics(final int characteristics) { > > final Shape shape = getTestShape(); > > final BloomFilter inner = new > > DefaultBloomFilterTest.SparseDefaultBloomFilter(shape) { > > @Override > > public int characteristics() { > > return characteristics; > > } > > }; > > final WrappedBloomFilter underTest = new > WrappedBloomFilter(inner) > > { > > @Override > > public BloomFilter copy() { > > final BloomFilter result = new > > DefaultBloomFilterTest.SparseDefaultBloomFilter(shape); > > result.merge(getWrapped()); > > return result; > > } > > }; > > assertEquals(characteristics, underTest.characteristics()); > > } > > > > > -- > LinkedIn: http://www.linkedin.com/in/claudewarren >