Migrate Solr tests from SolrJettyTestBase to using SolrJettyTestRule #3947

epugh · 2025-12-14T12:18:35Z

This is a bit of an experiment. I wanted to see if I could reduce the amount of effort to get our tests updated. Over time we've been accumulating tech debt in our tests, and addressing that manually appears pretty difficult!

In this PR I've checked in my prompt prompt_to_use.md that I used in VS Code. My thought is that the most important aspect of this PR is crafting a really great prompt. Instead of reviewing and commenting on the changes made per individual test file, what I really want is review of the overall change, and feed that review BACK into my prompt. Then I will re-run the improved prompt and that hopefully will output the final set of changes. Maybe in this PR, maybe in another fresh PR with improved prompt.

As an example of feeding back input, after running everything I noticed in one test it hardcoded a 127.0.0.1 url, and so I added Please do not create hard coded urls like 127.0.0.1 in the tests. to prevent that in the future.

There are three files that start with MIGRATION_*.md and they are somewhat messy AI generated status files. I wanted it to maintain tests_not_migrated.md as the but it kind of forgot that file....

…rTestCaseJ4 I used the prompt that I previously checked in, and pretty much just let it go to town. I had to help it a bit on the BasicHttpSolrClientTest. tests_not_migrated.md represents the ones that didn't go on the first pass.

…ious batch They all look pretty straightforward however.

… moving methods around.

This reverts commit c44be5d.

…code and moving methods around." This reverts commit 21f2949.

epugh · 2025-12-15T16:06:28Z

Some lessons learned:

Be more prescriptive on the process. After migrating the first ten or so, Chat wanted to migrate three or five classes at a time, instead of grinding them out one by one.
It's hard not to just engage with Chat to steer him to fix things. Which then made me feel more committed to getting this specific PR done... Versus my original plan of coming up with one "perfect" prompt and then just re run it multiple times till everything worked.
Chat really understands how multiple Java components talk to each and the flow in a way that takes me hours of stepping through code....

…olrjettytestbase

This reverts commit bf278de.

This reverts commit cab85b2.

dsmiley · 2025-12-19T17:20:02Z

Big changes like this should be a multi-step journey. Anticipate that and ask the AI where to start and just stay focused there. For example look at SolrJettyTestBase and focus on removing that. Perhaps even that might be too much for one PR, so narrow further on RestTestBae, for example.

…olrjettytestbase

epugh · 2025-12-28T17:21:25Z

Tracking my prompt here:

The class #file:solr/test-framework/src/java/org/apache/solr/SolrJettyTestBase.java has been deprecated in favour of using #file:solr/test-framework/src/java/org/apache/solr/util/SolrJettyTestRule.java.

I want to migrate our tests away from #file:solr/test-framework/src/java/org/apache/solr/SolrJettyTestBase.java.   Please go through the code base and migrate each test one by one.  I want you to migrate each test, running the unit test after each one.  If you can't successfully migrate it, then I want you to write it out to "tests_not_migrated.md" and move on.

Please look at #file:solr/core/src/test/org/apache/solr/response/TestPrometheusResponseWriter.java as an example of good use of #file:solr/test-framework/src/java/org/apache/solr/util/SolrJettyTestRule.java test rule.

Please do not change createTempDir() method to LuceneTestCase.createTempDir().

Please do not create hard coded urls like 127.0.0.1 in the tests.

We use the try-with-resources pattern to make sure resouces such as httpclients, solrclients, cores etc are closed.  Please use that where possible and skip manually closing the resource.  Unless of course we run into issues with the `ObjectReleaseTracker`.

…olrjettytestbase

Swapping from default configset to techproducts meant that I could just load system version!

…olrjettytestbase

…ClientTestBase.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

…h/solr into migrate_away_from_solrjettytestbase

epugh · 2025-12-29T16:54:02Z

At this point, I'm down to working on migrating how we create collections to follow the pattern that TestPrometheusWriter has and that I also used in DistributedDebugComponentTest. I could keep going, however I think maybe we should merge at this point, and then do that in the next one??? WDYT @dsmiley ?

epugh · 2025-12-29T17:14:41Z

Just discovered EmbeddedSolrServerTestBase, I guess it's next on the list to be migrated!

Migrate to Path, and wraps the few places we have a string value with Path.of. I suspect they may be refactored away in the future as well.

dsmiley

Thanks again for undertaking this!

Adding a couple more comments but I suppose this is in pretty good shape!

dsmiley · 2025-12-29T18:26:27Z

solr/core/src/test/org/apache/solr/handler/TestStressIncrementalBackup.java


-    // NOTE: we don't actually care about using SolrCloud, but we want to use SolrClient and I can't
-    // bring myself to deal with the nonsense that is SolrJettyTestBase.
+    // NOTE: we don't actually care about using SolrCloud, but we want to use SolrClient.


LOL. Maybe this becomes a TODO now that we have a nice rule encapsulating Solr lifecycle for Jetty
CC @gerlowskija

solr/test-framework/src/java/org/apache/solr/util/SolrClientTestRule.java

Copilot

Pull request overview

Copilot reviewed 82 out of 82 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-30T13:35:52Z

solr/test-framework/src/java/org/apache/solr/util/SolrClientTestRule.java

        if (configSet.endsWith(confSuffix)) {
          configSet = configSet.substring(0, configSet.length() - confSuffix.length());
        }
+        this.configSet = configSet;


The assignment this.configSet = configSet; has been moved inside the conditional block. If the configSet doesn't end with the confSuffix, this field will not be set, which could cause the configSet to remain uninitialized or keep its previous value.

maybe. I changed it so if you pass in a null you get a null.

Copilot · 2025-12-30T13:35:53Z

solr/core/src/test/org/apache/solr/update/CustomTLogDirTest.java


  @ClassRule
-  public static final SolrClientTestRule solrClientTestRule =
+  public static final SolrClientTestRule solrTestRule =


Inconsistent naming: This file uses solrClientTestRule for the rule name, while most other migrated files use solrTestRule. This inconsistency makes the codebase harder to maintain.

Copilot · 2025-12-30T13:35:53Z

solr/core/src/test/org/apache/solr/update/CustomTLogDirTest.java

        protected void before() {
          System.setProperty("solr.directoryFactory", "solr.NRTCachingDirectoryFactory");
-          solrClientTestRule.startSolr(LuceneTestCase.createTempDir());
+          solrTestRule.startSolr(LuceneTestCase.createTempDir());


The solrClientTestRule.startSolr() call is using the wrong rule name - it should be solrTestRule.startSolr() to match the field name on line 37.

…3947) Embrace the use of @ClassRule setup to manage the Solr lifecycle using SolrClientTestRule in our tests. This PR is to finish the migration to using SolrJettyTestRule, which extends SolrClientTestRule across our tests. There is also plenty of small cleanups of the tests throughout. Lint cleanups and places where we had extra setup steps that did not need to happen for the goal of a individual test to be accomplished.

hossman · 2025-12-30T21:26:03Z

DistributedDebugComponentTest seems to now be failing 100% of the time, regardless of seed

epugh · 2025-12-30T22:23:55Z

solr/core/src/test/org/apache/solr/handler/component/DistributedDebugComponentTest.java

  }

  @AfterClass
-  public static void destroyThings() throws Exception {


I wonder if this logic needs to be restored to fix the policeman build?

epugh added 4 commits December 13, 2025 09:23

First pass of a prompt for migrating to SolrJettyTestRule

ede5683

for what ever reason these were considered more complex then the prev…

fbfadce

…ious batch They all look pretty straightforward however.

Additional migration of tests.

652dc89

github-actions bot added test-framework jetty-server client:solrj module:sql tests cat:search cat:schema cat:metrics labels Dec 14, 2025

epugh added 7 commits December 14, 2025 07:56

add two more instructions based on review...

f2efcef

Pretty straightforward migraiton..

f3d688e

These migrations appear more concerning, a lot of deprecated code and…

21f2949

… moving methods around.

Actually remove SolrJettyTestBase!

c44be5d

tracking file updates

614c5f7

Revert "Actually remove SolrJettyTestBase!"

1177167

This reverts commit c44be5d.

Revert "These migrations appear more concerning, a lot of deprecated …

835d926

…code and moving methods around." This reverts commit 21f2949.

epugh added 6 commits December 15, 2025 13:26

Merge remote-tracking branch 'upstream/main' into migrate_away_from_s…

33c61ce

…olrjettytestbase

remove files that dont have license for now

bf278de

remove files that dont have license for now

cab85b2

Revert "remove files that dont have license for now"

df7ba8d

This reverts commit bf278de.

Revert "remove files that dont have license for now"

e2270f6

This reverts commit cab85b2.

add license header just to pass precommit, they are temp files however

5de956c

dsmiley marked this pull request as draft December 19, 2025 17:20

epugh added 2 commits December 22, 2025 07:04

Merge remote-tracking branch 'upstream/main' into migrate_away_from_s…

dff81d7

…olrjettytestbase

Update dependencies needed for tests.

793d987

epugh changed the title ~~[EXPERIMENT!] Use AI to migrate Solr tests from SolrJettyTestBase to using SolrJettyTestRule~~ Migrate Solr tests from SolrJettyTestBase to using SolrJettyTestRule Dec 28, 2025

remove in progress generated files

12aa2da

github-actions bot removed the tool:build label Dec 28, 2025

epugh and others added 11 commits December 28, 2025 13:29

Merge remote-tracking branch 'upstream/main' into migrate_away_from_s…

0c1a6e4

…olrjettytestbase

Using the SolrClientTestRule.create for collections.

583db85

Swapping from default configset to techproducts meant that I could just load system version!

Use modern approach for creating the needed collection.

cd8c40f

Merge remote-tracking branch 'upstream/main' into migrate_away_from_s…

3344164

…olrjettytestbase

Update solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpSolr…

e258844

…ClientTestBase.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Clean up that David spotted that applies in many places!

796bac2

Merge branch 'migrate_away_from_solrjettytestbase' of github.com:epug…

1851cd5

…h/solr into migrate_away_from_solrjettytestbase

Inline deprecated method

a2a66d9

try with resoruces is som much shorter

d24a74c

lint

d610b04

Standardize on solrTestRule across the code base.

2b9010f

github-actions bot added module:ltr cat:index labels Dec 29, 2025

Except in four places, we always have a Path. So let's embrace the Path.

4a46298

Migrate to Path, and wraps the few places we have a string value with Path.of. I suspect they may be refactored away in the future as well.

dsmiley reviewed Dec 29, 2025

View reviewed changes

epugh added 2 commits December 30, 2025 06:55

Back out overly aggressive swap to path, and allow string configset

e37bbb4

two lints while checking for a potential method to remove

6b590b1

epugh requested a review from Copilot December 30, 2025 13:25

Copilot started reviewing on behalf of epugh December 30, 2025 13:26 View session

Copilot AI reviewed Dec 30, 2025

View reviewed changes

epugh merged commit 6b82523 into apache:main Dec 30, 2025
10 of 12 checks passed

epugh commented Dec 30, 2025

View reviewed changes

Migrate Solr tests from SolrJettyTestBase to using SolrJettyTestRule #3947

Migrate Solr tests from SolrJettyTestBase to using SolrJettyTestRule #3947

Uh oh!

Conversation

epugh commented Dec 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

epugh commented Dec 15, 2025

Uh oh!

dsmiley commented Dec 19, 2025

Uh oh!

epugh commented Dec 28, 2025

Uh oh!

epugh commented Dec 29, 2025

Uh oh!

epugh commented Dec 29, 2025

Uh oh!

dsmiley left a comment

Choose a reason for hiding this comment

Uh oh!

dsmiley Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

epugh Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hossman commented Dec 30, 2025

Uh oh!

epugh Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

epugh commented Dec 14, 2025 •

edited

Loading