[
https://issues.apache.org/jira/browse/LUCENE-3501?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13123717#comment-13123717
]
Shai Erera commented on LUCENE-3501:
------------------------------------
I briefly went through the patch:
* In the test, I prefer to still catch Exception, or if you want to be on the
safe side, Throwable. And have assertSameResult throw RuntimeException. Calling
fail() from there, forcing you to handle Errors, seem too low-level to me ...
* In AdaptiveFacetsAccumulator you have this line: //private Sampler sampler =
new RepeatableSampler();. Is it a leftover?
* Maybe add a line or two to RandomSample.createSample() (internal comments),
such as:
** "Skip over 'step' documents" before the for-loop
** "Add all leftover documents to the sampled set" before last 'while'.
(Please also confirm my understanding) :)
* CHANGES - in this case you only need a CHANGES entry for 3x (since the change
is applied to there as well), and it is under contrib.
Otherwise, it looks very good !
> random sampler is not random (and so facet SamplingWrapperTest occasionally
> fails)
> ----------------------------------------------------------------------------------
>
> Key: LUCENE-3501
> URL: https://issues.apache.org/jira/browse/LUCENE-3501
> Project: Lucene - Java
> Issue Type: Bug
> Components: modules/facet
> Reporter: Doron Cohen
> Assignee: Doron Cohen
> Priority: Minor
> Attachments: LUCENE-3501.patch
>
>
> RandomSample is not random at all:
> It does not even import java.util.Random, and its behavior is deterministic.
> in addition, the test testCountUsingSamping() never retries as it was
> supposed to (for taking care of the hoped-for randomness).
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
