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
>

Reply via email to