Conversation
Co-authored-by: epugh <22395+epugh@users.noreply.github.com> Agent-Logs-Url: https://github.com/epugh/querqy-solr/sessions/28060663-7353-44fa-be05-eadba18a88a4
…ctQuerqySolrCloudTestCase Co-authored-by: epugh <22395+epugh@users.noreply.github.com> Agent-Logs-Url: https://github.com/epugh/querqy-solr/sessions/eef48999-e12c-4944-a7d2-0428268ce51d
Co-authored-by: epugh <22395+epugh@users.noreply.github.com> Agent-Logs-Url: https://github.com/epugh/querqy-solr/sessions/c8fb41d9-46ae-4429-92da-688c4708bc04
Co-authored-by: epugh <22395+epugh@users.noreply.github.com> Agent-Logs-Url: https://github.com/epugh/querqy-solr/sessions/f722468c-bd6e-471a-988f-73ef0dd80e1b
…solr runtime deps Co-authored-by: epugh <22395+epugh@users.noreply.github.com> Agent-Logs-Url: https://github.com/epugh/querqy-solr/sessions/035d4a2f-a691-4a61-b4ba-a7370141cdbf
We have an issue with Solr 10's test framework and ExternalPaths, which may mean we have to wait for Solr 10.1 to get our tests here working. I tested against a local patched that we called solr 11, and that shows up in the pom.xml.
epugh
left a comment
epugh
left a comment
There was a problem hiding this comment.
First self-review of this PR. So much work to do...
| "name", this.rewriterConfigIndexName)); | ||
|
|
||
| try (final LocalSolrQueryRequest solrUpdateRequest = new LocalSolrQueryRequest(configurationCore, requestParams)) { | ||
| try (final SolrQueryRequestBase solrUpdateRequest = new SolrQueryRequestBase(configurationCore, requestParams) {}) { |
There was a problem hiding this comment.
I wonder why we didn't create a non abstract version of SolrQueryRequestBase when we did this so we didn't have the {} anonymous class thing?
| assert null != client.getDefaultCollection(); | ||
| AbstractDistribZkTestBase.waitForRecoveriesToFinish(client.getDefaultCollection(), cluster.getZkStateReader(), | ||
| true, true, 330); | ||
| AbstractFullDistribZkTestBase.waitForRecoveriesToFinish( |
There was a problem hiding this comment.
not sure why true,true in solr 9, and false,true in solr 10..
| * - Solr 10.0: Would fail with NPE during class loading | ||
| * - Solr 10.1+: Should handle null SOURCE_HOME gracefully | ||
| */ | ||
| public class ExternalPathsCompatibilityTest { |
There was a problem hiding this comment.
too verbose AI generated, and do we need all of this?
| import org.apache.solr.SolrTestCaseJ4; | ||
| import org.apache.solr.client.solrj.SolrServerException; | ||
| import org.apache.solr.client.solrj.impl.HttpSolrClient; | ||
| import org.apache.solr.client.solrj.apache.HttpSolrClient; |
There was a problem hiding this comment.
we should move to jetty. Not apache.
There was a problem hiding this comment.
This class appears to have been mostly not run as part of the tests... So maybe back it out?
| @@ -0,0 +1 @@ | |||
| // placeholder for ExternalPaths.determineSourceHome() | |||
| <properties> | ||
| <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding> | ||
| <java.version>11</java.version> | ||
| <java.version>21</java.version> |
There was a problem hiding this comment.
solr minimum version on the server side, i think client is still 17.
| <log4j.version>2.21.1</log4j.version> | ||
| <slf4j.version>2.0.13</slf4j.version> | ||
| <solr.version>9.8.0</solr.version> | ||
| <solr.version>11.0.0-SNAPSHOT</solr.version> |
There was a problem hiding this comment.
this should be 10.0, but I amde my own patched version to teal with solr-test-framework issues.
There was a problem hiding this comment.
way too long due to horked up poms from solr 10.0 ;-(
|
@renekrie was hoping to have this done done before you got home as a "welcome back" gift, but didn't quite get there. |
|
Turns out apache/solr#4225 was already open.... |
This is very much a WIP, with lots of copilot and Claude help.
Right now this is the current test results:
Tests run: 349, Failures: 1, Errors: 17, Skipped: 1.Some things to note:
Solr 10 introduced several breaking changes that affected querqy-solr:
Solr 10.0 has a broken
solr-test-framework.jar, the ExternalPaths.java class raises an NPE instead of a null. I have a thread open on dev@ mailing list and a potential fix.I suspect to move forward we'll need to wait till 10.1 comes ;-(.