Ignore my previous message. I don't know where my mind was. On Sat, Oct 5, 2024 at 4:26 PM 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. > > 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 > -- LinkedIn: http://www.linkedin.com/in/claudewarren