[GitHub] [solr] dsmiley commented on pull request #1374: SOLR-16623:SolrClientTestRule for JettySolrRunnerV2

["dsmiley (via GitHub)" <gi...@apache.org>]

dsmiley commented on PR #1374:
URL: https://github.com/apache/solr/pull/1374#issuecomment-1440449203

   FYI Eric/others this is a re-do PR to reduce the scope of change to JettySolrTestBase & subclasses.  Too much work to fix/improve them in greater detail.
   
   From our online review today:
   * Exception handling is wrong; details were discussed
   * SolrClientTestRule.getSolrClient (and any overloads) should be documented to say "The caller doesn't need to close it."  And furthermore, the method _may_ return the same instance for the same collection.
   * SolrJettyTestBase shouldn't have a field for the SolrClient, just as it doesn't have a field for Jetty either (thanks to your change).  The client's lifecycle (create & close) should be managed by the rule.
   * In the rule (perhaps even in SolrClientTestRule!), add a ConcurrentHashMap to track the clients by their collection name.  Thankfully (due to Eric Pugh's heroism), clients are mostly immutable thus we can return the same instance.  Obviously they should be closed out in `after()`.


-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org