[EXPERIMENT!] Use AI to 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-22T13:54:10Z

Note for later:

This pattern if often NOT needed. Remove it and the test passes...

Files.copy(SolrTestCaseJ4.TEST_HOME().resolve("solr.xml"), homeDir.resolve("solr.xml"));

Also, look for src_dir, taht isn't a java pattern and is places where I think we can reduce lines of setup by using helper methods.

Also, make sure System.clearProperty("solr.test.sys.prop2"); is needed... I think a lot of tests don't need it.

epugh · 2025-12-22T14:49:30Z

I think I am there for this effort. I didn't manage to move ALL clasess from SolrJettyTestBase, we still have classes that extend it... CacheHeaderTestBase, and RestTestBase. Having said that, the rest of classes being migrated here I think are ready for final review!

I thought I would come up with a prompt, run it, review the results and edit the prompt and re-run it. That didn't really happen... And I ended up going through each class and putting a lot of manual review in and tweaking from what Claude first did!

epugh · 2025-12-22T14:51:38Z

Please ignore the documentation related files, I will remove them as part of my final review.. I'm keeping them mostly as "working file"....

epugh · 2025-12-23T02:31:44Z

Actually, SolrExampleTestBase turned out to be not so bad to migrate!

dsmiley · 2025-12-23T05:50:55Z

solr/test-framework/src/test/org/apache/solr/client/solrj/apache/BasicHttpSolrClientTest.java

This entire package is going away soon.

I very much look forward to that!

dsmiley · 2025-12-23T05:59:56Z

solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTestsBase.java

-public abstract class SolrExampleTestsBase extends SolrJettyTestBase {
+public abstract class SolrExampleTestsBase extends SolrTestCaseJ4 {
+
+  @ClassRule public static SolrJettyTestRule solrJettyTestRule = new SolrJettyTestRule();


perhaps we should standardize on a field name for the rule. Like "solrRule" is fairly short & sweet. We needn't put it's "Jetty"-ness in the name.

Sure! Would you want to rename the class SolrJettyTestRule to SolrTestRule too?

Sure! Would you want to rename the class SolrJettyTestRule to SolrTestRule too?

No of course not; Jetty is what differentiates that class from EmbeddedSolrServerTestRule, and eventually from a forthcoming SolrCloudTestRule. An analogy would be using ArrayList but declaring a field/param/var as list.

dsmiley · 2025-12-23T06:01:31Z

solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTestsBase.java

+    return getHttpSolrClient(solrJettyTestRule.getBaseUrl(), DEFAULT_TEST_CORENAME);
+  }
+
+  public static HttpSolrClient getHttpSolrClient(String baseUrl, String coreName) {


It's apparently public in the parent class SolrTestCaseJ4 so I can't...

i amc hecking if we duplicate methods from parent classes..

Now I see that this method was copied from STCJ4. Why copy it?

dsmiley · 2025-12-23T06:10:03Z

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

-    // create second core
-    try (SolrClient nodeClient = getHttpSolrClient(url)) {
-      CoreAdminRequest.Create req = new CoreAdminRequest.Create();
-      req.setCoreName("collection2");
-      req.setConfigSet("collection1");
-      nodeClient.request(req);
-    }


what happened to the 2nd core?

We now do set up directly:

**Old approach **: Start Solr with one core, then use CoreAdminRequest.Create to add the second core dynamically

New approach: Pre-create both core directories and configs, so Solr discovers and loads both cores during startup

Claude says this is fine and doesn't defeat the purpose of the test. Follows our more common patterns as well.

good qeustion to ask!

Is this test not amenable to SolrJettyTestRule.create or maybe Claude/you didn't know it exists because of too few callers? Some tests may not be but I haven't examined this one closely to know. When I created the Solr rule APIs, I wanted to ensure it has a generalized API for core/collection creation so that we could have some tests minimally change between an (eventual) SolrCloud vs Jetty vs simply embedded. I also wanted tests to look more "normal" with respect to SolrClient API. Pre-creation is kind of low level and also questionable with SolrCloud. If we were to lean in the other direction (pre-creation), I'd want to see more/better solr-home dir setup methods. That wouldn't be a bad thing anyway.
@hossman maybe you might have an opinion as an old-timer on test infra here, if I can be so lucky to get your attention on the GitHub platform ;-).

So the approach you are doing is not wrong. But I was trying to get away from, not attract to. It's a "preference" matter, definitely not a strong deprecate/going-away.

dsmiley · 2025-12-23T06:17:23Z

solr/core/src/test/org/apache/solr/servlet/ResponseHeaderTest.java

+        }
      }
+      assertTrue("Expected header not found", containsWarningHeader);
+      HttpClientUtil.close(httpClient);


the try-with-resources means you don't need this explicit close

Copilot

Pull request overview

This PR is an experimental effort to use AI to migrate Solr tests from the deprecated SolrJettyTestBase to using SolrJettyTestRule. The primary goal is to reduce technical debt by modernizing test infrastructure, with the focus on refining the AI prompt (prompt_to_use.md) rather than reviewing individual test file changes.

Key changes:

Migration of numerous test classes from extending SolrJettyTestBase to SolrTestCaseJ4 with @ClassRule SolrJettyTestRule
Updates to base test classes (SolrExampleTestsBase, HttpSolrClientTestBase) to support the new pattern
Replacement of legacy methods (getBaseUrl(), getSolrClient(), etc.) with rule-based equivalents
Documentation of migration progress and patterns in markdown files

Reviewed changes

Copilot reviewed 40 out of 40 changed files in this pull request and generated 11 comments.

Show a summary per file

File	Description
prompt_to_use.md	AI prompt defining migration requirements and patterns
tests_not_migrated.md	Documentation of migration status and remaining work
MIGRATION_*.md	AI-generated status and progress tracking files
solr/test-framework/.../BasicHttpSolrClientTest.java	Migrated to use SolrJettyTestRule with extensive method call replacements
solr/test-framework/.../ConcurrentUpdate*.java	Multiple test classes migrated to new pattern
solr/solrj/.../HttpSolrClientTestBase.java	Base class updated with DEFAULT_COLLECTION constant and rule support
solr/solrj/.../SolrExampleTestsBase.java	Base class refactored with deprecated backward-compatibility methods
solr/solrj/.../*Test.java	Various test classes migrated following standard pattern
solr/core/.../ResponseHeaderTest.java	Migrated with improved resource management using try-with-resources
solr/core/.../TestTolerantSearch.java	Complex multi-collection test migrated with custom setup
solr/core/.../DistributedDebugComponentTest.java	Multi-collection test migrated
solr/modules/sql/.../TestSQLHandlerNonCloud.java	Migrated with minimal config setup
solr/test-framework/.../SolrTestCaseJ4.java	Added documentation to copyMinConf method

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

Copilot · 2025-12-23T16:36:08Z

solr/solrj/src/test/org/apache/solr/client/solrj/SolrSchemalessExampleTest.java

+  protected HttpClient getHttpClient() {
+    HttpSolrClient client = (HttpSolrClient) getSolrClient();
+    return client.getHttpClient();
+  }


The method getHttpClient() is added but is never called within this file. If this is an override of a parent class method or an interface method, it should be annotated with @Override. Otherwise, consider removing this unused method or documenting its purpose.

Copilot · 2025-12-23T16:36:09Z

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

+    collection1 = null;
+    collection2 = null;


In the migration to SolrJettyTestRule, the clients collection1 and collection2 are no longer explicitly closed in the AfterClass method. While the comment suggests relying on the rule for cleanup, verify that the SolrJettyTestRule properly closes all SolrClient instances obtained via getSolrClient() to prevent resource leaks.

Suggested change

collection1 = null;

collection2 = null;

if (collection1 != null) {

collection1.close();

collection1 = null;

}

if (collection2 != null) {

collection2.close();

collection2 = null;

}

Copilot · 2025-12-23T16:36:09Z

solr/modules/sql/src/test/org/apache/solr/handler/sql/TestSQLHandlerNonCloud.java

+    Path workDir = createTempDir().toRealPath();
+    Path collectionDirectory = workDir.resolve(DEFAULT_TEST_COLLECTION_NAME);
+
+    copyMinConf(collectionDirectory, "name=" + DEFAULT_TEST_COLLECTION_NAME + "\n");
+
    return workDir;


The migration removes the setupJettyTestHome call and replaces it with copyMinConf. However, setupJettyTestHome may have been copying additional configuration files beyond what copyMinConf provides. Verify that all necessary configuration files (such as schema.xml, managed-schema, etc.) are still properly copied for the test to function correctly.

Copilot · 2025-12-23T16:36:10Z

solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTestsBase.java

+  // Backward compatibility methods for existing subclasses
+  @Deprecated
+  protected static void createAndStartJetty(Path solrHome) throws Exception {
+    solrJettyTestRule.startSolr(solrHome, new Properties(), JettyConfig.builder().build());
  }


The deprecated method createAndStartJetty is being added to SolrExampleTestsBase but marked as deprecated. This suggests subclasses may still be calling it. Consider removing this method entirely and updating all subclasses to use the new pattern with solrJettyTestRule.startSolr() instead of maintaining backward compatibility with a deprecated API.

Suggested change

// Backward compatibility methods for existing subclasses

@Deprecated

protected static void createAndStartJetty(Path solrHome) throws Exception {

solrJettyTestRule.startSolr(solrHome, new Properties(), JettyConfig.builder().build());

}

solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTestsBase.java

solr/core/src/test/org/apache/solr/TestTolerantSearch.java

Copilot · 2025-12-23T16:36:11Z

solr/core/src/test/org/apache/solr/TestTolerantSearch.java

+    collection1 = null;
+    collection2 = null;


The clients collection1 and collection2 in TestTolerantSearch are set to null in AfterClass without being closed. This creates a potential resource leak. Either close these clients before nulling them, or verify that the SolrJettyTestRule handles cleanup of all client instances.

Suggested change

collection1 = null;

collection2 = null;

if (collection1 != null) {

collection1.close();

collection1 = null;

}

if (collection2 != null) {

collection2.close();

collection2 = null;

}

Copilot · 2025-12-23T16:36:11Z

solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpSolrClientTestBase.java

-  protected static final String DEFAULT_CORE = "foo";
+  @ClassRule public static SolrJettyTestRule solrClientTestRule = new SolrJettyTestRule();
+
+  protected static final String DEFAULT_COLLECTION = "collection1";


The constant DEFAULT_CORE has been renamed to DEFAULT_COLLECTION. However, this constant is being used throughout the test methods which may indicate an incomplete migration. Consider verifying that all references to the old constant name have been updated consistently. Additionally, DEFAULT_COLLECTION should likely use the same value as DEFAULT_TEST_CORENAME for consistency across the codebase.

Suggested change

protected static final String DEFAULT_COLLECTION = "collection1";

protected static final String DEFAULT_COLLECTION = DEFAULT_TEST_CORENAME;

Copilot · 2025-12-23T16:36:11Z

solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTests.java

 @SuppressSSL
 public abstract class SolrExampleTests extends SolrExampleTestsBase {
  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());



The static block that calls ignoreException("uniqueKey") has been removed. If this exception ignore was serving a purpose in handling expected exceptions during tests, its removal could cause test failures or unexpected error logging. Verify that this removal is intentional and doesn't affect test behavior.

Suggested change

static {

ignoreException("uniqueKey");

}

Copilot · 2025-12-23T16:36:12Z

solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTestsBase.java

+  protected SolrClient getSolrClient() {
    if (client == null) {
      client = createNewSolrClient();
    }
    return client;
  }


The getSolrClient method visibility has changed from public (with @Override) to protected (without @Override). This is a breaking change for any code that was relying on this being a public method. If this method is part of a base class contract, verify that all subclasses and callers can still access it appropriately.

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 added 5 commits December 22, 2025 08:24

rework awkward comment.

abaeda2

lint

13c4ee1

Using copyMinConf reduces boilerplate copying.

0577c46

With the migration, some of the comments are out of date.

afdf1c1

simplify client setup.

8ef50c4

epugh added 2 commits December 22, 2025 08:57

unneeded code

19f746d

Code review driven tweaks.

758d698

epugh added the no-changelog label Dec 22, 2025

unneeded property set/clear

0905bb0

epugh marked this pull request as ready for review December 22, 2025 14:49

epugh requested review from dsmiley and iamsanjay December 22, 2025 14:49

epugh added 4 commits December 22, 2025 13:49

Migrate SolrExampleTestsBase to using SolrJettyTestRule

9be336b

move method to specific subclass that uses it.

e246dab

Remove unused code

2e624aa

Various code clean ups from looking for issues.

978e884

epugh added 2 commits December 22, 2025 21:42

basic migation done

633c342

Update names and lint checking of java code

f71725b

dsmiley reviewed Dec 23, 2025

View reviewed changes

epugh added 3 commits December 23, 2025 08:02

Commit changes before merge

f0ea4c8

respond to feedback.

a8c6187

Remove a method duplicated by the parent class.

9dbe91a

epugh requested review from Copilot and removed request for iamsanjay December 23, 2025 16:25

Copilot started reviewing on behalf of epugh December 23, 2025 16:25 View session

Copilot AI reviewed Dec 23, 2025

View reviewed changes

	protected static final String DEFAULT_COLLECTION = "collection1";
	protected static final String DEFAULT_COLLECTION = DEFAULT_TEST_CORENAME;

+  static {
+    ignoreException("uniqueKey");
+  }

[EXPERIMENT!] Use AI to migrate Solr tests from SolrJettyTestBase to using SolrJettyTestRule #3947

Are you sure you want to change the base?

[EXPERIMENT!] Use AI to migrate Solr tests from SolrJettyTestBase to using SolrJettyTestRule #3947

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 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

epugh commented Dec 22, 2025

Uh oh!

epugh commented Dec 23, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

epugh commented Dec 14, 2025 •

edited

Loading

epugh commented Dec 22, 2025 •

edited

Loading

epugh commented Dec 22, 2025 •

edited

Loading