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

Reply via email to