>
> 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

Reply via email to