epugh commented on PR #3708: URL: https://github.com/apache/solr/pull/3708#issuecomment-3675012365
Okay, I made the change in `SolrTestCaseHS`. I asked Claude about doing the same in `SolrTestCaseJ4` and he said: Should we make the same change?** **Actually, no!** Here's why the patterns in `SolrTestCaseJ4.java` are **different and appropriate**: ### ✅ **Why SolrTestCaseJ4 patterns are correct as-is:** 1. **Core-level testing**: `SolrTestCaseJ4` is designed for core-level unit testing, not client-level integration testing 2. **Direct handler invocation**: Methods like `updateJ()` and `TestHarness.query()` directly invoke handlers, which is the appropriate test level 3. **No manual response conversion**: They use `req.getResponseWriter().writeToString()`, which is the **correct** way to get string responses from core operations 4. **Performance appropriate**: For core-level testing, avoiding the client serialization overhead is actually desirable ### ❌ **Why SolrTestCaseHS needed the change:** The patterns we fixed in `SolrTestCaseHS` and `TestHarness.update()` were problematic because they: - Created requests through `EmbeddedSolrServer` - Got `NamedList` responses - **Then manually converted them back to XML** using `XMLResponseWriter` This was **double work** - serializing to get a NamedList, then serializing again to get XML. ## 🎯 **Conclusion:** **No, we should NOT make the same change in `SolrTestCaseJ4.java`** because: 1. **Different purpose**: `SolrTestCaseJ4` is for core-level testing, `SolrTestCaseHS` is for client-level testing 2. **Correct patterns**: The current patterns in `SolrTestCaseJ4` use appropriate core-level APIs 3. **Performance optimal**: Direct core operations are faster than going through the client layer 4. **No anti-patterns**: There are no problematic manual XML conversion patterns to fix The optimizations we made were specifically for cases where: - Code was using `EmbeddedSolrServer` to get responses - Then manually converting those responses to XML/JSON strings `SolrTestCaseJ4` doesn't have this anti-pattern, so the changes aren't needed or beneficial there. **Our optimizations were surgical fixes for specific inefficiencies, not blanket replacements of all core-level testing!** 🎯 -- 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]
