[VOTE] Release Apache Commons Parent 77 based on RC1
We have fixed a few bugs and added enhancements since Apache Commons Parent 76 was released, so I would like to release Apache Commons Parent 77. Apache Commons Parent 77 RC1 is available for review here: https://dist.apache.org/repos/dist/dev/commons/parent/77-RC1 (svn revision 72148) The Git tag commons-parent-77-RC1 commit for this RC is 80a06beec388a53716605075a35c8ce03e922b3d which you can browse here: https://gitbox.apache.org/repos/asf?p=commons-parent.git;a=commit;h=80a06beec388a53716605075a35c8ce03e922b3d You may checkout this tag using: git clone https://gitbox.apache.org/repos/asf/commons-parent.git --branch commons-parent-77-RC1 commons-parent-77-RC1 Maven artifacts are here: https://repository.apache.org/content/repositories/orgapachecommons-1782/org/apache/commons/commons-parent/77/ These are the artifacts and their hashes: #Release SHA-512s #Sat Oct 05 19:08:22 UTC 2024 commons-parent-77-bom.json=28648a6f96857b61542d92a165dcd932105bc33668db5a27b2b1d7d5c8509926c802fcca3b8f1118e32ea6c70379dc471995a5431672b4074b5141ba7ecfd321 commons-parent-77-bom.xml=9ad5bd716356981e42b07f7f0c6023e42952070152dced3c2d8691b4d3c67d5b614508d90ee988fb31d5148345113b253bb7fba6a680b35adaa234818e9e46fb commons-parent-77-site.xml=67d93a8bd57621ef1ff9b3576e8708fe2e95a9e33ca3c37959dec286ebb14f518a987d4e20d72e4bc595bf7a623794c864ffca3202dfe02c227caad37b5a7bd8 commons-parent-77-src.tar.gz=bc693c6c9333b1576fe4c565fd36376b295cd27000fba515c4f2b0aa6fb975dd8ee82bf2caa72ce7afd06ffed9c81f6b405e45d1b601044e056357afabb78616 commons-parent-77-src.zip=2e2689d68dae093cc1ab56eabd2bf0d8056d143a3888deb26e779a79dbc29ed3a2b6cbeb52599641ae4df10175553d37cf4d15c77675567f019b92fe7afc9cb1 org.apache.commons_commons-parent-77.spdx.json=3bd8f74e92d2834b381c6080d9a29e5c025a2dbf25fa61dbbec44a8b881d8551aea5f7e982b3f28abd8dd65a7e315172a3ad0516191c3be02afa7a67156d87b0 I have tested this with 'mvn' and 'mvn -e -V -Prelease -Ptest-deploy -P jacoco -P japicmp clean package site deploy' using: openjdk version "17.0.12" 2024-07-16 OpenJDK Runtime Environment Homebrew (build 17.0.12+0) OpenJDK 64-Bit Server VM Homebrew (build 17.0.12+0, mixed mode, sharing) Apache Maven 3.9.9 (8e8579a9e76f7d015ee5ec7bfcdc97d260186937) Maven home: /usr/local/Cellar/maven/3.9.9/libexec Java version: 17.0.12, vendor: Homebrew, runtime: /usr/local/Cellar/openjdk@17/17.0.12/libexec/openjdk.jdk/Contents/Home Default locale: en_US, platform encoding: UTF-8 OS name: "mac os x", version: "15.0.1", arch: "x86_64", family: "mac" Darwin 24.0.0 Darwin Kernel Version 24.0.0: Tue Sep 24 23:36:30 PDT 2024; root:xnu-11215.1.12~1/RELEASE_X86_64 x86_64 Details of changes since 76 are in the release notes: https://dist.apache.org/repos/dist/dev/commons/parent/77-RC1/RELEASE-NOTES.txt https://dist.apache.org/repos/dist/dev/commons/parent/77-RC1/site/changes-report.html Site: https://dist.apache.org/repos/dist/dev/commons/parent/77-RC1/site/index.html (note some *relative* links are broken and the 77 directories are not yet created - these will be OK once the site is deployed.) RAT Report: https://dist.apache.org/repos/dist/dev/commons/parent/77-RC1/site/rat-report.html KEYS: https://downloads.apache.org/commons/KEYS Please review the release candidate and vote. This vote will close no sooner than 72 hours from now. [ ] +1 Release these artifacts [ ] +0 OK, but... [ ] -0 OK, but really should fix... [ ] -1 I oppose this release because... Thank you, Gary Gregory, Release Manager (using key 86fdc7e2a11262cb) The following is intended as a helper and refresher for reviewers. Validating a release candidate == These guidelines are NOT complete. Requirements: Git, Java, Maven. You can validate a release from a release candidate (RC) tag as follows. 1a) Clone and checkout the RC tag git clone https://gitbox.apache.org/repos/asf/commons-parent.git --branch commons-parent-77-RC1 commons-parent-77-RC1 cd commons-parent-77-RC1 1b) Download and unpack the source archive from: https://dist.apache.org/repos/dist/dev/commons/parent/77-RC1/source 2) Check Apache licenses This step is not required if the site includes a RAT report page which you then must check. mvn apache-rat:check 3) Check binary compatibility Older components still use Apache Clirr: This step is not required if the site includes a Clirr report page which you then must check. mvn clirr:check Newer components use JApiCmp with the japicmp Maven Profile: This step is not required if the site includes a JApiCmp report page which you then must check. mvn install -DskipTests -P japicmp japicmp:cmp 4) Build the package mvn -V clean package You can record the Maven and Java version produced by -V in your VOTE reply. To gather OS information from a command line: Windows: ver Linux: uname -a 5) Build the site for a single module project Note: Some plugins require the components to be installed instead of packaged. mvn site Check the site reports in: -
Re: [collections] BloomFilterExtractor.flatten()
On Fri, Oct 4, 2024 at 1:30 PM Alex Herbert wrote: > You cannot return an empty BloomFilter when there is no array of > BloomFilters to flatten. The BloomFilter is tied to a Shape. Without a > shape you cannot know what size filter to create. So we have to return > null or throw an exception. Whatever choice must be documented. > The code now throws an exception instead of returning null. I also updated the Javadoc. > > Note we do not currently check if the Shapes are all equal when > creating the BloomFilterExtractor. This is not essential but when > merging two Bloom filters in flatten an error will be thrown if the > filters are not compatible. I say it is not essential as the > functionality of the BloomFilterExtract in processBloomFilters does > not mandate any prerequisites on the Predicate for the filters. So for > example you could have many filters of different sizes and a predicate > that tests if they all contain a given IndexProducer. > I'll leave that deeper topic alone for now, and for you and Claude to further discuss if warranted. TY, Gary > > Alex > > On Fri, 4 Oct 2024 at 18:02, Gary Gregory wrote: > > > > OK, but how do you create an empty filter? > > > > Gary > > > > On Fri, Oct 4, 2024, 12:54 PM Claude Warren wrote: > > > > > I think that it should return an empty filter as the javadoc says > > > > > > > > > On Fri 4 Oct 2024, 15:53 Gary D. Gregory, wrote: > > > > > > > See the new disabled test BloomFilterExtractorTest. > > > > > > > > Gary > > > > > > > > On 2024/10/04 14:51:55 "Gary D. Gregory" wrote: > > > > > Hi Claude and all, > > > > > > > > > > The method: > > > > > > > > > > > > > > > > > > org.apache.commons.collections4.bloomfilter.BloomFilterExtractor.flatten() > > > > > > > > > > is documented as always returning a BloomFilter: > > > > > > > > > > /** > > > > > * Create a standard (non-layered) Bloom filter by merging all > of > > > > the layers. If > > > > > * the filter is empty this method will return an empty Bloom > > > filter. > > > > > * > > > > > * @return the merged bloom filter. > > > > > */ > > > > > > > > > > But that's not how it's coded, it can return null, the simplest > > > > reproducer is: > > > > > > > > > > BloomFilterExtractor.fromBloomFilterArray(new > BloomFilter[0]).flatten() > > > > -> null > > > > > > > > > > Should we: > > > > > > > > > > - Change the Javadoc > > > > > - Change the code > > > > > -- How should this code be changed? > > > > > -- Should the method be moved to BloomFilter and use "this" as the > > > > default filter? > > > > > > > > > > TY, > > > > > Gary > > > > > > > > > > > - > > > > > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > > > > > For additional commands, e-mail: dev-h...@commons.apache.org > > > > > > > > > > > > > > > > > > - > > > > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > > > > For additional commands, e-mail: dev-h...@commons.apache.org > > > > > > > > > > > > > - > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > >
Re: [collections] Does WrappedBloomFilterTest make sense?
On Sat, Oct 5, 2024 at 11:46 AM Claude Warren wrote:
> Your Fixture solution is close to correct.
>
> @Override
> > public WrappedBloomFilter copy() {
> > return new Fixture(getWrapped());
> > }
>
>
> should return new Fixture(getWrapped().copy());
>
Thank you for your review Claude. These changes are now in git master.
Gary
>
> On Sat, Oct 5, 2024 at 4:42 PM Claude Warren wrote:
>
> > Ignore my previous message. I don't know where my mind was.
> >
> > On Sat, Oct 5, 2024 at 4:26 PM Claude Warren 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
> >> 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
> >>
Re: [collections] Does WrappedBloomFilterTest make sense?
On Sat, Oct 5, 2024 at 11:26 AM Claude Warren 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
> 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
>
[collections] Does WrappedBloomFilterTest make sense?
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());
}
Re: [collections] Does WrappedBloomFilterTest make sense?
>
> 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 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
Re: [collections] Does WrappedBloomFilterTest make sense?
Ignore my previous message. I don't know where my mind was.
On Sat, Oct 5, 2024 at 4:26 PM Claude Warren 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
> 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
Re: [collections] Does WrappedBloomFilterTest make sense?
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 wrote:
> Ignore my previous message. I don't know where my mind was.
>
> On Sat, Oct 5, 2024 at 4:26 PM Claude Warren 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
>> 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
