Deal with properties that are needed for ONE test, but pollute all our tests.

[epugh]

Description

Through out our tests in setup/teardown we deal with solr.test.sys.prop1 and solr.test.sys.prop2... However, only a single test ACTUALLY cares about that property.

In my rough count, there are 33 places we do clean up, 32 which don't care about it!

Solution

Seperate out a specific test that cares, and then go through and clean up various files that carry this messy baggage.

Tests

Rerun tests

[epugh]

Removed more lines than we added... And simplified our test plumbing! Pretty happy about this, and it's ready for review.

[Copilot]

Pull request overview

This PR addresses test pollution by isolating system property usage that was only needed by a single test but was being set globally across 33 different test locations. The solution creates a dedicated test class TestConfigPropertySubstitution for the one test that actually requires the solr.test.sys.prop1 and solr.test.sys.prop2 properties, and removes the unnecessary property setup/teardown from 32 other test locations.

Key changes:

• Created dedicated TestConfigPropertySubstitution test class with isolated property setup for testing configuration property substitution
• Removed global property setup/teardown from 32+ test files and framework classes
• Created minimal solrconfig-test-properties.xml configuration file specifically for property substitution testing

Reviewed changes

Copilot reviewed 29 out of 29 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
solr/core/src/test/org/apache/solr/core/TestConfigPropertySubstitution.java	New dedicated test class for property substitution testing with isolated setup
solr/core/src/test-files/solr/collection1/conf/solrconfig-test-properties.xml	New minimal config file containing only the propTest element for isolated testing
solr/core/src/test/org/apache/solr/core/TestConfig.java	Removed testJavaProperty test method (moved to new dedicated class) and cleaned up unused imports
solr/core/src/test-files/solr/collection1/conf/solrconfig-test-misc.xml	Removed propTest element no longer needed in this shared config
solr/test-framework/src/java/org/apache/solr/util/TestHarness.java	Removed global property setup that polluted all tests
solr/test-framework/src/java/org/apache/solr/cloud/AbstractZkTestCase.java	Removed unnecessary property cleanup
solr/test-framework/src/java/org/apache/solr/cloud/AbstractFullDistribZkTestBase.java	Removed unnecessary property setup and cleanup
solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java	Removed global property setup
solr/test-framework/src/java/org/apache/solr/SolrJettyTestBase.java	Removed unnecessary property setup and TODO comment
solr/test-framework/src/java/org/apache/solr/BaseDistributedSearchTestCase.java	Removed unnecessary property setup
solr/modules/extraction/src/test-files/extraction/solr/collection1/conf/solrconfig.xml	Removed propTest element from extraction module config
solr/test-framework/src/test-files/solr/collection1/conf/solrconfig.xml	Removed propTest element from test framework config
solr/core/src/test/org/apache/solr/update/processor/AbstractAtomicUpdatesMultivalueTestBase.java	Removed property setup with TODO comments
solr/core/src/test/org/apache/solr/update/RootFieldTest.java	Removed property setup with TODO comments
solr/core/src/test/org/apache/solr/search/stats/TestDistribIDF.java	Removed unnecessary property setup and cleanup
solr/core/src/test/org/apache/solr/search/join/ShardToShardJoinAbstract.java	Removed property setup/cleanup, made class abstract, cleaned up unused imports and variables
solr/core/src/test/org/apache/solr/schema/PrimitiveFieldTypeTest.java	Removed unnecessary property setup
solr/core/src/test/org/apache/solr/schema/DateFieldTest.java	Removed unnecessary property setup
solr/core/src/test/org/apache/solr/rest/SolrRestletTestBase.java	Removed unnecessary property setup
solr/core/src/test/org/apache/solr/metrics/SolrMetricsIntegrationTest.java	Removed unnecessary property setup
solr/core/src/test/org/apache/solr/handler/ReplicationTestHelper.java	Removed unnecessary property setup
solr/core/src/test/org/apache/solr/core/TestCorePropertiesReload.java	Removed unnecessary property setup
solr/core/src/test/org/apache/solr/cloud/TestRequestForwarding.java	Removed unnecessary property setup and cleanup
solr/core/src/test-files/solr/collection1/conf/solrconfig.xml	Removed propTest element from main collection1 config
solr/core/src/test-files/solr/collection1/conf/solrconfig-plugcollector.xml	Removed propTest element from plugcollector config
solr/core/src/test-files/solr/collection1/conf/solrconfig-minhash.xml	Removed propTest element from minhash config
solr/core/src/test-files/solr/collection1/conf/solrconfig-elevate.xml	Removed propTest element from elevate config
solr/core/src/test-files/solr/collection1/conf/solrconfig-collapseqparser.xml	Removed propTest element from collapseqparser config
solr/core/src/test-files/solr/collection1/conf/solrconfig-analytics-query.xml	Removed propTest element from analytics-query config

---

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

[Copilot]

The comment references TestConfig.testJavaProperty() but this test method was moved to TestConfigPropertySubstitution.testJavaPropertySubstitution() in the new dedicated test class. The comment should be updated to reference the correct test class and method.

Suggested change

	This config is used by TestConfig.testJavaProperty() to test that system property
	substitution works correctly in Solr configuration files.
	This config is used by TestConfigPropertySubstitution.testJavaPropertySubstitution() to test
	that system property substitution works correctly in Solr configuration files.