epugh commented on PR #4369: URL: https://github.com/apache/solr/pull/4369#issuecomment-4326714907
I have tagged @dsmiley for review. I did look at the tests and they appear very "mock heavy", and we tend as a project to try and minimize the use of mocks where possible, b/c many of our bugs occur when real objects interact with other real objects ;-). There are a lot of mock objects in the tests so I asked copilot about this and got this response: > Yes—very likely. If the tests for this change are “mock-heavy”, you can usually cover this behavior with one of these lower-mock options (often with zero mocks), because the behavior you’re asserting is fundamentally an HTTP/response mapping concern: when Lucene/Solr query parsing/search throws a specific IllegalStateException (FieldExistsQuery “requires …”), Solr should return 400 BAD_REQUEST instead of 500. > 1) Best option: a Solr integration-style test (no mocks) Write a test that boots a minimal SolrCore (SolrTestCaseJ4-style) with a tiny schema and then issues a request that triggers the FieldExistsQuery requires ... exception. Assert: HTTP status code is 400 (or SolrException code == BAD_REQUEST depending on harness) error message contains the “FieldExistsQuery requires …” text. This approach tests the real wiring through QueryComponent, not an artificial mocked SolrIndexSearcher / ResponseBuilder graph. Could you take a stab at reducing/removing the use of mocks? I know QueryComponent can be tough to work with directly because of how it sets up state through ResponseBuilders etc... If a non mock version isn't feasible, then it's better to have this test, even if it's brittle! -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
