Previously the tests took 1m20s to run, now they take 2m55s. I guess it's because previously we only created RAMDirs, while now newDirectory picks FSDir from time to time (10%?).
I don't think its from FSDir, this is now very very rarely picked. Anyway, as said in the issue summary, for a number of reasons, I don't want to address this on this issue, I want to address the coverage first.
FacetTestUtils.close*() can be removed and calls replaced by IOUtils.closeSafely. This is not critical, just remove redundant code.
ah, you are right. let's change this.
You added a TODO to CategoryListIteratorTest about the test failing if TieredMP is used. In general TieredMP is not good for the taxonomy index, which relies on Lucene doc IDs, and therefore segments must be merged in-order. LTW uses LMP specifically because of that. I will look into the test to understand why would it care about doc IDs, since it doesn't using the taxonomy index at all.
Right, as you said this is for the main index, not the taxonomy index. So I think the test just relies upon lucene doc ids, but I didnt want to just change the test without saying why.
There are few places with code like: assertTrue("Would like to test this with deletions!",indexReader.hasDeletions()), and assertTrue("Would like to test this with deletions!",indexReader.numDeletedDocs() > 0) which you removed. Any reason?
Mostly to prevent the tests from failing. RandomIndexWriter randomly optimizes some times, so occasionally there are no deletions. I think this is fine (actually better) as far as coverage... then the deleted docs is occasionally null, etc.
You added a TODO to TestScoredDocIDsUtils (about reader is read-only) – you're right, the comment can be deleted.
OK, I'll nuke this.
We can add RandomTaxonomyWriter as a follow-on commit.
Yes, lets do this separate.