gerlowskija commented on a change in pull request #665: Fixes SOLR-13539
URL: https://github.com/apache/lucene-solr/pull/665#discussion_r296343329
##########
File path: solr/test-framework/src/java/org/apache/solr/SolrJettyTestBase.java
##########
@@ -140,29 +139,17 @@ public synchronized SolrClient getSolrClient() {
}
/**
- * Create a new solr client.
- * If createJetty was called, an http implementation will be created,
- * otherwise an embedded implementation will be created.
- * Subclasses should override for other options.
+ * Create a new solr client. If createJetty was called, an http
implementation will be created, otherwise an embedded
+ * implementation will be created. Subclasses should override for other
options.
*/
public SolrClient createNewSolrClient() {
- if (jetty != null) {
- try {
- // setup the client...
- String url = jetty.getBaseUrl().toString() + "/" + "collection1";
- HttpSolrClient client = getHttpSolrClient(url,
DEFAULT_CONNECTION_TIMEOUT);
- return client;
- }
- catch( Exception ex ) {
- throw new RuntimeException( ex );
- }
- } else {
- return new EmbeddedSolrServer( h.getCoreContainer(), "collection1" ) {
- @Override
- public void close() {
- // do not close core container
- }
- };
+ try {
+ // setup the client...
+ final String url = jetty.getBaseUrl().toString() + "/" + "collection1";
+ final HttpSolrClient client = getHttpSolrClient(url,
DEFAULT_CONNECTION_TIMEOUT);
+ return client;
+ } catch (final Exception ex) {
+ throw new RuntimeException(ex);
Review comment:
[-1] Either I missed this on an earlier review, or this change is new in the
latest iteration of this PR. (Publishing changes with git's "force-push"
option as you've been doing makes it hard to see which changes are new and
which have been around for a few iterations).
Anyway, I didn't notice this change previously, but some test failures drew
my attention to it.
When I check out your PR and merge master into it locally, I get several
test failures:
```
[junit4] Tests with failures [seed: D067023E71C9E8C8]:
[junit4] - org.apache.solr.update.RootFieldTest.testUpdateWithChildDocs
[junit4] -
org.apache.solr.update.RootFieldTest.testLegacyBlockProcessing
[junit4] -
org.apache.solr.handler.tagger.EmbeddedSolrNoSerializeTest.testAssertTagStreamingWithSolrTaggerRequest
[junit4] -
org.apache.solr.handler.tagger.EmbeddedSolrNoSerializeTest.testTag
[junit4] -
org.apache.solr.update.processor.AbstractAtomicUpdatesMultivalueTest.initializationError
```
I looked into the RootFieldTest failure, and it is failing because of an NPE
in this method:
```
[junit4] ERROR 0.01s | RootFieldTest.testLegacyBlockProcessing <<<
[junit4] > Throwable #1: java.lang.RuntimeException:
java.lang.NullPointerException
[junit4] > at
__randomizedtesting.SeedInfo.seed([84EEE01E1A0D0DB8:E5D4F6A2DBEAF389]:0)
[junit4] > at
org.apache.solr.SolrJettyTestBase.createNewSolrClient(SolrJettyTestBase.java:152)
[junit4] > at
org.apache.solr.SolrJettyTestBase.getSolrClient(SolrJettyTestBase.java:136)
[junit4] > at
org.apache.solr.update.RootFieldTest.testLegacyBlockProcessing(RootFieldTest.java:58)
[junit4] > at
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
[junit4] > at
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
[junit4] > at
java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
[junit4] > at
java.base/java.lang.reflect.Method.invoke(Method.java:566)
[junit4] > at java.base/java.lang.Thread.run(Thread.java:834)
[junit4] > Caused by: java.lang.NullPointerException
[junit4] > at
org.apache.solr.SolrJettyTestBase.createNewSolrClient(SolrJettyTestBase.java:148)
[junit4] > ... 41 more
```
Was there a reason you removed the `jetty != null` check from this block,
and the corresponding `else` block that created an EmbeddedSolrServer if jetty
hadn't been called? It seems like those are still needed here.
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
