Your Fixture solution is close to correct.
@Override
> public WrappedBloomFilter copy() {
> return new Fixture(getWrapped());
> }
should return new Fixture(getWrapped().copy());
On Sat, Oct 5, 2024 at 4:42 PM Claude Warren <[email protected]> wrote:
> Ignore my previous message. I don't know where my mind was.
>
> On Sat, Oct 5, 2024 at 4:26 PM Claude Warren <[email protected]> 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 <[email protected]>
>> 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
>
--
LinkedIn: http://www.linkedin.com/in/claudewarren
