HoustonPutman commented on code in PR #2436:
URL: https://github.com/apache/solr/pull/2436#discussion_r1586713299


##########
solr/test-framework/src/java/org/apache/solr/cloud/SolrCloudTestCase.java:
##########
@@ -140,29 +138,18 @@ public static void shutdownCluster() throws Exception {
   @BeforeClass
   public static void setPrsDefault() {
     Class<?> target = RandomizedTest.getContext().getTargetClass();
-    if (target != null && target.isAnnotationPresent(NoPrs.class)) return;
-    if (isPRS()) {
-      System.setProperty("solr.prs.default", "true");
+    boolean prsDefault;
+    if (target != null && target.isAnnotationPresent(NoPrs.class)) {
+      prsDefault = false;
+    } else {
+      prsDefault = random().nextBoolean();

Review Comment:
   Done, and added another sysProp to save the user-provided value that we 
might be overwriting because of Class annotations



##########
solr/test-framework/src/java/org/apache/solr/cloud/SolrCloudTestCase.java:
##########
@@ -83,7 +83,7 @@ public class SolrCloudTestCase extends SolrTestCaseJ4 {
 
   private static final Logger log = 
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
-  public static final String PRS_DEFAULT_PROP = 
System.getProperty("use.per-replica", null);
+  public static final String PRS_DEFAULT_PROP = "solr.prs.default";

Review Comment:
   Put it in Solr where it is used.



##########
solr/test-framework/src/java/org/apache/solr/cloud/SolrCloudTestCase.java:
##########
@@ -97,11 +97,9 @@ protected static SolrZkClient zkClient() {
     return cluster.getZkStateReader().getZkClient();
   }
 
-  /** if the system property is not specified, use a random value */
+  /** if the system property is not specified, default to false. The 
SystemProperty will be set in a beforeClass method. */
   public static boolean isPRS() {
-    return PRS_DEFAULT_PROP == null
-        ? random().nextBoolean()
-        : Boolean.parseBoolean(PRS_DEFAULT_PROP);
+    return Boolean.parseBoolean(System.getProperty(PRS_DEFAULT_PROP, "false"));

Review Comment:
   Used it, in all places, but really in the tests, it will only be cared about 
in the `setPrsDefault()` since we overwrite the sysProp always.



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to