mikemccand commented on pull request #128: URL: https://github.com/apache/lucene/pull/128#issuecomment-847864069
> One thing I'm still researching is that, it seems there's limited direct test coverage for this `CheckIndex` class? I see there's `TestCheckIndex`, but it only has 4 tests, and the majority of its functionalities seems to be put under tests by other index testing utilities and test cases. Shall I still add a few more tests for these changes (and should I put them in `TestCheckIndex`)? On the other hand, I've been running `./gradlew check -Ptests.nightly=true -Pvalidation.git.failOnModified=false` the whole time and all tests have been passing. More tests are always welcome! But I don't think that should block this change -- we can open a follow-on issue to get better direct unit tests for `CheckIndex`? Those would be fun tests to write: make a healthy index, then make a random single bit change to one of its files, and then see if `CheckIndex` catches it. Hmm I think we have such a test somewhere :) But not apparently in `BaseTestCheckIndex`... -- 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: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
