leerho commented on a change in pull request #324:
URL:
https://github.com/apache/incubator-datasketches-java/pull/324#discussion_r452619693
##########
File path: src/test/java/org/apache/datasketches/theta/UnionImplTest.java
##########
@@ -54,14 +54,14 @@ public void checkUpdateWithSketch() {
assertEquals(union.getResult().getEstimate(), k, 0.0);
}
- @Test(expectedExceptions = SketchesArgumentException.class)
- public void checkCorruptedCompactFlag() {
+ @Test
+ public void checkUnorderedCompactFlag() {
int k = 16;
WritableMemory mem = WritableMemory.wrap(new byte[(k*8) + 24]);
UpdateSketch sketch =
Sketches.updateSketchBuilder().setNominalEntries(k).build();
for (int i=0; i<k; i++) { sketch.update(i); }
CompactSketch sketchInDirectOrd = sketch.compact(true, mem);
- sketch.compact(false, mem); //corrupt memory
+ sketch.compact(false, mem); //change the order bit
Review comment:
My you have a very good eye! At first I thought you found a bug, but
what you found was an inert test, which I have now improved, thanks!
As of version 1.3.0, there was an additional layer of classes that were
ordered and unordered subclasses of the HeapCompactSketch and the
DirectCompactSketch. But this was really unnecessary. So I eliminated those 4
classes as they only differed by the ordered bit. So before, there were two
ways to determine whether the CompactSketch was ordered or not: the class type,
and the ordered bit (memory) or boolean (on heap). This test was designed to
test the situation where the indicators were out-of-sync and make sure an error
was thrown.
With the new version there is only one indicator of order, either a boolean
on heap or a bit set in Memory.
As a result this test was inert. So instead of removing it, I added
additional checks to make sure that the union operation would successfully and
correctly merge the same hashes whether the sketch order flag was set or not.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
