You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2022/08/09 06:36:18 UTC

[GitHub] [commons-collections] Claudenw opened a new pull request, #328: Collections 827: Add tests using or, and and xor with different length filters.

Claudenw opened a new pull request, #328:
URL: https://github.com/apache/commons-collections/pull/328

   Added tests between shapes of different length for "or", "and" and "xor" operations.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [commons-collections] aherbert merged pull request #328: Collections 827: Add tests using or, and and xor with different length filters.

Posted by GitBox <gi...@apache.org>.
aherbert merged PR #328:
URL: https://github.com/apache/commons-collections/pull/328


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [commons-collections] aherbert commented on a diff in pull request #328: Collections 827: Add tests using or, and and xor with different length filters.

Posted by GitBox <gi...@apache.org>.
aherbert commented on code in PR #328:
URL: https://github.com/apache/commons-collections/pull/328#discussion_r941542308


##########
src/test/java/org/apache/commons/collections4/bloomfilter/SetOperationsTest.java:
##########
@@ -222,12 +222,26 @@ public final void testOrCardinality() {
         filter2 = new SparseBloomFilter(shape, IndexProducer.fromIndexArray(new int[] { 5, 64, 69 }));
         assertEquals(4, SetOperations.orCardinality(filter1, filter2));
         assertEquals(4, SetOperations.orCardinality(filter2, filter1));
+    }
 
-        Shape bigShape = Shape.fromKM(3, 192);
-        filter1 = new SparseBloomFilter(bigShape, IndexProducer.fromIndexArray(new int[] { 1, 63, 185}));
-        filter2 = new SparseBloomFilter(shape, IndexProducer.fromIndexArray(new int[] { 5, 63, 69 }));
+    @Test
+    public final void testOrCardinalityWithDifferentLengthFilters() {
+        Shape shape = Shape.fromKM(3, 128);
+        Shape shape2 = Shape.fromKM(3, 192);
+        SparseBloomFilter filter1 = new SparseBloomFilter(shape, IndexProducer.fromIndexArray(new int[] { 1, 63, 64 }));
+        SparseBloomFilter filter2 = new SparseBloomFilter(shape2, IndexProducer.fromIndexArray(new int[] { 5, 64, 169 }));
+        assertEquals(5, SetOperations.orCardinality(filter1, filter2));
+        assertEquals(5, SetOperations.orCardinality(filter2, filter1));
+
+        filter1 = new SparseBloomFilter(shape, IndexProducer.fromIndexArray(new int[] { 1, 63 }));
+        filter2 = new SparseBloomFilter(shape2, IndexProducer.fromIndexArray(new int[] { 5, 64, 169 }));
         assertEquals(5, SetOperations.orCardinality(filter1, filter2));
         assertEquals(5, SetOperations.orCardinality(filter2, filter1));
+
+        filter1 = new SparseBloomFilter(shape, IndexProducer.fromIndexArray(new int[] { 5, 63 }));
+        filter2 = new SparseBloomFilter(shape2, IndexProducer.fromIndexArray(new int[] { 5, 64, 169 }));
+        assertEquals(4, SetOperations.orCardinality(filter1, filter2));
+        assertEquals(4, SetOperations.orCardinality(filter2, filter1));

Review Comment:
   I think we can reduce the amount of duplicate code with a custom assertion:
   ```Java
           assertSymmetricOperation(4, SetOperations::orCardinality, filter1, filter2);
       }
   
       private static void assertSymmetricOperation(int expected, ToIntBiFunction<BloomFilter, BloomFilter> operation,
               BloomFilter filter1, BloomFilter filter2) {
           assertEquals(expected, operation.applyAsInt(filter1, filter2), "op(filter1, filter2)");
           assertEquals(expected, operation.applyAsInt(filter2, filter1), "op(filter2, filter1)");
       }
   ```
   
   This ensures you only have one place where you specify the expected value and where filter 1 and 2 are swapped as arguments to the assertion.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [commons-collections] codecov-commenter commented on pull request #328: Collections 827: Add tests using or, and and xor with different length filters.

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #328:
URL: https://github.com/apache/commons-collections/pull/328#issuecomment-1208974618

   # [Codecov](https://codecov.io/gh/apache/commons-collections/pull/328?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#328](https://codecov.io/gh/apache/commons-collections/pull/328?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4c58624) into [master](https://codecov.io/gh/apache/commons-collections/commit/df091173cdfabd5ecc852f47c978ee9bcb2b7059?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (df09117) will **decrease** coverage by `0.05%`.
   > The diff coverage is `n/a`.
   
   ```diff
   @@             Coverage Diff              @@
   ##             master     #328      +/-   ##
   ============================================
   - Coverage     86.01%   85.96%   -0.06%     
   + Complexity     4675     4673       -2     
   ============================================
     Files           288      288              
     Lines         13473    13473              
     Branches       1980     1980              
   ============================================
   - Hits          11589    11582       -7     
   - Misses         1323     1327       +4     
   - Partials        561      564       +3     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/commons-collections/pull/328?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...commons/collections4/map/AbstractReferenceMap.java](https://codecov.io/gh/apache/commons-collections/pull/328/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2NvbW1vbnMvY29sbGVjdGlvbnM0L21hcC9BYnN0cmFjdFJlZmVyZW5jZU1hcC5qYXZh) | `88.88% <0.00%> (-2.60%)` | :arrow_down: |
   
   :mega: Codecov can now indicate which changes are the most critical in Pull Requests. [Learn more](https://about.codecov.io/product/feature/runtime-insights/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org