dnhatn commented on pull request #134: URL: https://github.com/apache/lucene/pull/134#issuecomment-839934931
@rmuir @jpountz Thanks for reviewing. > Can we add tests to BaseStoredFieldsFormatTestCase that call setIndexSort, exercise merging, and validate correctness (e.g. check that stored fields values are still correct)? I am worried that existing test coverage for this change is not great. I added a random test in https://github.com/apache/lucene/pull/134/commits/be7c71c320be86f5e485a6d0864b18954f5ef25f. > Can we also fix merge logic of Lucene90TermVectorsFormat/BaseTermVectorsFormatTestCase? Generally, we try to keep this logic consistent across stored fields and term vectors as much as possible, and this improvement should apply there as well? +1. Are you okay if I do will it in a follow-up right after this PR? Would you please take another look? -- 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