You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2022/02/24 22:05:34 UTC

[GitHub] [solr] risdenk commented on a change in pull request #704: SOLR-14920: Spotless formatting for test-framework

risdenk commented on a change in pull request #704:
URL: https://github.com/apache/solr/pull/704#discussion_r814301498



##########
File path: solr/test-framework/src/java/org/apache/solr/cloud/AbstractMoveReplicaTestBase.java
##########
@@ -236,15 +254,21 @@ public void test() throws Exception {
     }
     assertTrue("replica never fully recovered", recovered);
 
-    assertEquals(100, cluster.getSolrClient().query(coll, new SolrQuery("*:*")).getResults().getNumFound());
+    assertEquals(
+        100, cluster.getSolrClient().query(coll, new SolrQuery("*:*")).getResults().getNumFound());
   }
 
-  //Commented out 5-Dec-2017
+  // Commented out 5-Dec-2017
   // @AwaitsFix(bugUrl = "https://issues.apache.org/jira/browse/SOLR-11458")
   @Test
-  // 12-Jun-2018 @BadApple(bugUrl="https://issues.apache.org/jira/browse/SOLR-12028") // 17-Mar-2018 This JIRA is fixed, but this test still fails
-  //17-Aug-2018 commented  @LuceneTestCase.BadApple(bugUrl="https://issues.apache.org/jira/browse/SOLR-12028") // 2-Aug-2018
-  // commented out on: 17-Feb-2019   @BadApple(bugUrl="https://issues.apache.org/jira/browse/SOLR-12028") // annotated on: 24-Dec-2018
+  // 12-Jun-2018 @BadApple(bugUrl="https://issues.apache.org/jira/browse/SOLR-12028") // 17-Mar-2018
+  // This JIRA is fixed, but this test still fails
+  // 17-Aug-2018 commented
+  // @LuceneTestCase.BadApple(bugUrl="https://issues.apache.org/jira/browse/SOLR-12028") //
+  // 2-Aug-2018
+  // commented out on: 17-Feb-2019
+  // @BadApple(bugUrl="https://issues.apache.org/jira/browse/SOLR-12028") // annotated on:
+  // 24-Dec-2018

Review comment:
       Fix this

##########
File path: solr/test-framework/src/java/org/apache/solr/cloud/api/collections/AbstractCollectionsAPIDistributedZkTestBase.java
##########
@@ -359,32 +405,36 @@ public void testCreateNodeSet() throws Exception {
   }
 
   @Test
-  //28-June-2018 @BadApple(bugUrl="https://issues.apache.org/jira/browse/SOLR-12028")
-  // See: https://issues.apache.org/jira/browse/SOLR-12028 Tests cannot remove files on Windows machines occasionally
-  // commented out on: 24-Dec-2018   @BadApple(bugUrl="https://issues.apache.org/jira/browse/SOLR-12028") // added 09-Aug-2018 SOLR-12028
+  // 28-June-2018 @BadApple(bugUrl="https://issues.apache.org/jira/browse/SOLR-12028")
+  // See: https://issues.apache.org/jira/browse/SOLR-12028 Tests cannot remove files on Windows
+  // machines occasionally
+  // commented out on: 24-Dec-2018
+  // @BadApple(bugUrl="https://issues.apache.org/jira/browse/SOLR-12028") // added 09-Aug-2018
+  // SOLR-12028

Review comment:
       Fix this

##########
File path: solr/test-framework/src/java/org/apache/solr/cloud/ChaosMonkey.java
##########
@@ -53,27 +52,28 @@
 
 /**
  * The monkey can stop random or specific jetties used with SolrCloud.
- * 
- * It can also run in a background thread and start and stop jetties
- * randomly.
- * TODO: expire multiple sessions / connectionloss at once
- * TODO: kill multiple jetties at once
- * TODO: ? add random headhunter mode that always kills the leader
- * TODO: chaosmonkey should be able to do cluster stop/start tests
+ *
+ * <p>It can also run in a background thread and start and stop jetties randomly. TODO: expire
+ * multiple sessions / connectionloss at once TODO: kill multiple jetties at once TODO: ? add random
+ * headhunter mode that always kills the leader TODO: chaosmonkey should be able to do cluster
+ * stop/start tests

Review comment:
       Fix this?

##########
File path: solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java
##########
@@ -2216,32 +2338,40 @@ public static void copyXmlToHome(File dstRoot, String fromFile) throws IOExcepti
     Files.createDirectories(dstRoot.toPath());
     Files.copy(SolrTestCaseJ4.TEST_PATH().resolve(fromFile), dstRoot.toPath().resolve("solr.xml"));
   }
-  // Creates a consistent configuration, _including_ solr.xml at dstRoot. Creates collection1/conf and copies
+  // Creates a consistent configuration, _including_ solr.xml at dstRoot. Creates collection1/conf
+  // and copies
   // the stock files in there.

Review comment:
       Fix this

##########
File path: solr/test-framework/src/java/org/apache/solr/cloud/AbstractMoveReplicaTestBase.java
##########
@@ -98,7 +101,9 @@ public void afterTest() throws Exception {
   }
 
   @Test
-  // commented out on: 17-Feb-2019   @BadApple(bugUrl="https://issues.apache.org/jira/browse/SOLR-12028") // annotated on: 24-Dec-2018
+  // commented out on: 17-Feb-2019
+  // @BadApple(bugUrl="https://issues.apache.org/jira/browse/SOLR-12028") // annotated on:
+  // 24-Dec-2018

Review comment:
       Fix this

##########
File path: solr/test-framework/src/test/org/apache/solr/util/TestSSLTestConfig.java
##########
@@ -83,22 +82,25 @@ public void testIsOpenJdkJvmVersionKnownToHaveProblems() {
       final String v = "13" + suffix;
       assertTrue(v, SSLTestConfig.isOpenJdkJvmVersionKnownToHaveProblems(v));
     }
-    
   }
 
   public void testFailIfUserRunsTestsWithJVMThatHasKnownSSLBugs() {
-    // NOTE: If there is some future JVM version, where all available "ea" builds are known to be buggy,
-    // but we still want to be able to use for running tests (ie: via jenkins) to look for *other* bugs,
+    // NOTE: If there is some future JVM version, where all available "ea" builds are known to be
+    // buggy,
+    // but we still want to be able to use for running tests (ie: via jenkins) to look for *other*
+    // bugs,

Review comment:
       Fix this

##########
File path: solr/test-framework/src/test/org/apache/solr/TestLogLevelAnnotations.java
##########
@@ -27,36 +27,35 @@
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
 
-@SuppressForbidden(reason="We need to use log4J2 classes to access the log levels")
-@LogLevel("org.apache.solr.bogus_logger.ClassLogLevel=error;org.apache.solr.bogus_logger.MethodLogLevel=warn")
+@SuppressForbidden(reason = "We need to use log4J2 classes to access the log levels")
+@LogLevel(
+    "org.apache.solr.bogus_logger.ClassLogLevel=error;org.apache.solr.bogus_logger.MethodLogLevel=warn")
 public class TestLogLevelAnnotations extends SolrTestCaseJ4 {
 
   private static final String bogus_logger_prefix = "org.apache.solr.bogus_logger";
-  
-  /** 
-   * <p>
-   * The default log level of the root logger when this class is loaded by the JVM, Used to validate 
+
+  /**
+   * The default log level of the root logger when this class is loaded by the JVM, Used to validate
    * some logger configurations when the tests are run.
-   * </p>
-   * <p>
-   * We don't want a hardcoded assumption here because it may change based on how the user runs the test.
-   * </p>
-   * <p>
-   * We also don't want to initialize this in a <code>@BeforeClass</code> method because that will run 
-   * <em>after</em> the <code>@BeforeClass</code> logic of our super class {@link SolrTestCaseJ4} 
-   * where the <code>@LogLevel</code> annotation on this class will be parsed and evaluated -- modifying the
-   * log4j run time configuration.  
-   * There is no reason why the <code>@LogLevel</code> configuration of this class <em>should</em> affect 
-   * the "root" Logger, but setting this in static class initialization protect us (as best we can) 
-   * against the possibility that it <em>might</em> due to an unforseen (future) bug.
-   * </p>
+   *
+   * <p>We don't want a hardcoded assumption here because it may change based on how the user runs
+   * the test.
+   *
+   * <p>We also don't want to initialize this in a <code>@BeforeClass</code> method because that
+   * will run <em>after</em> the <code>@BeforeClass</code> logic of our super class {@link
+   * SolrTestCaseJ4} where the <code>@LogLevel</code> annotation on this class will be parsed and
+   * evaluated -- modifying the log4j run time configuration. There is no reason why the <code>
+   * @LogLevel</code> configuration of this class <em>should</em> affect the "root" Logger, but
+   * setting this in static class initialization protect us (as best we can) against the possibility
+   * that it <em>might</em> due to an unforseen (future) bug.

Review comment:
       Fix this

##########
File path: solr/test-framework/src/java/org/apache/solr/cloud/AbstractBasicDistributedZkTestBase.java
##########
@@ -184,15 +184,17 @@ protected void setDistributedParams(ModifiableSolrParams params) {
 
   @Test
   @ShardsFixed(num = 4)
-  // commented out on: 17-Feb-2019   @BadApple(bugUrl="https://issues.apache.org/jira/browse/SOLR-12028") // annotated on: 24-Dec-2018
+  // commented out on: 17-Feb-2019
+  // @BadApple(bugUrl="https://issues.apache.org/jira/browse/SOLR-12028") // annotated on:
+  // 24-Dec-2018

Review comment:
       Fix this

##########
File path: solr/test-framework/src/test/org/apache/solr/TestLogLevelAnnotations.java
##########
@@ -65,31 +64,38 @@ public static void checkLogLevelsBeforeClass() {
     final LoggerContext ctx = (LoggerContext) LogManager.getContext(false);
     final Configuration config = ctx.getConfiguration();
 
-    // NOTE: we're checking the CONFIGURATION of the loggers, not the "effective" value of the Logger
-    assertEquals("Somehow, the configured value of the root logger changed since this class was loaded",
-                 DEFAULT_LOG_LEVEL, config.getRootLogger().getLevel());
-    assertEquals("Your Logger conf sets a level on a bogus package that breaks this test: "
-                 + bogus_logger_prefix,
-                 config.getRootLogger(),
-                 config.getLoggerConfig(bogus_logger_prefix));
-    assertEquals(Level.ERROR, config.getLoggerConfig(bogus_logger_prefix + ".ClassLogLevel").getLevel());
-    assertEquals(Level.WARN, config.getLoggerConfig(bogus_logger_prefix + ".MethodLogLevel").getLevel());
+    // NOTE: we're checking the CONFIGURATION of the loggers, not the "effective" value of the
+    // Logger
+    assertEquals(
+        "Somehow, the configured value of the root logger changed since this class was loaded",
+        DEFAULT_LOG_LEVEL,
+        config.getRootLogger().getLevel());
+    assertEquals(
+        "Your Logger conf sets a level on a bogus package that breaks this test: "
+            + bogus_logger_prefix,
+        config.getRootLogger(),
+        config.getLoggerConfig(bogus_logger_prefix));
+    assertEquals(
+        Level.ERROR, config.getLoggerConfig(bogus_logger_prefix + ".ClassLogLevel").getLevel());
+    assertEquals(
+        Level.WARN, config.getLoggerConfig(bogus_logger_prefix + ".MethodLogLevel").getLevel());
 
     // Now sanity check the EFFECTIVE Level of these loggers before the methods run...
     assertEquals(DEFAULT_LOG_LEVEL, LogManager.getRootLogger().getLevel());
     assertEquals(DEFAULT_LOG_LEVEL, LogManager.getLogger(bogus_logger_prefix).getLevel());
-    assertEquals(Level.ERROR, LogManager.getLogger(bogus_logger_prefix + ".ClassLogLevel").getLevel());
-    assertEquals(Level.WARN, LogManager.getLogger(bogus_logger_prefix + ".MethodLogLevel").getLevel());
+    assertEquals(
+        Level.ERROR, LogManager.getLogger(bogus_logger_prefix + ".ClassLogLevel").getLevel());
+    assertEquals(
+        Level.WARN, LogManager.getLogger(bogus_logger_prefix + ".MethodLogLevel").getLevel());
   }
 
-  /** 
+  /**
    * Check that the expected log level <em>configurations</em> have been reset after the test
-   * <p>
-   * <b>NOTE:</b> We only validate <code>@LogLevel</code> modifications made at the 
-   * {@link #testMethodLogLevels} level,  not at the 'class' level, because of the lifecycle of junit 
-   * methods: This <code>@AfterClass</code> will run before the <code>SolrTestCaseJ4</code> 
-   * <code>@AfterClass</code> method where the 'class' <code>@LogLevel</code> modifications will be reset.
-   * </p>
+   *
+   * <p><b>NOTE:</b> We only validate <code>@LogLevel</code> modifications made at the {@link
+   * #testMethodLogLevels} level, not at the 'class' level, because of the lifecycle of junit
+   * methods: This <code>@AfterClass</code> will run before the <code>SolrTestCaseJ4</code> <code>
+   * @AfterClass</code> method where the 'class' <code>@LogLevel</code> modifications will be reset.

Review comment:
       Fix this

##########
File path: solr/test-framework/src/java/org/apache/solr/cloud/AbstractMoveReplicaTestBase.java
##########
@@ -236,15 +254,21 @@ public void test() throws Exception {
     }
     assertTrue("replica never fully recovered", recovered);
 
-    assertEquals(100, cluster.getSolrClient().query(coll, new SolrQuery("*:*")).getResults().getNumFound());
+    assertEquals(
+        100, cluster.getSolrClient().query(coll, new SolrQuery("*:*")).getResults().getNumFound());
   }
 
-  //Commented out 5-Dec-2017
+  // Commented out 5-Dec-2017
   // @AwaitsFix(bugUrl = "https://issues.apache.org/jira/browse/SOLR-11458")

Review comment:
       Fix this

##########
File path: solr/test-framework/src/java/org/apache/solr/cloud/AbstractRecoveryZkTestBase.java
##########
@@ -62,14 +59,16 @@ public void stopThreads() throws InterruptedException {
   }
 
   @Test
-  //commented 2-Aug-2018 @BadApple(bugUrl="https://issues.apache.org/jira/browse/SOLR-12028") // 28-June-2018
+  // commented 2-Aug-2018 @BadApple(bugUrl="https://issues.apache.org/jira/browse/SOLR-12028") //
+  // 28-June-2018

Review comment:
       Fix this

##########
File path: solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java
##########
@@ -3022,26 +3180,26 @@ private static void randomizeNumericTypesProperties() {
       private_RANDOMIZED_NUMERIC_FIELDTYPES.put(Double.class, "solr.DoublePointField");
       private_RANDOMIZED_NUMERIC_FIELDTYPES.put(Date.class, "solr.DatePointField");
       private_RANDOMIZED_NUMERIC_FIELDTYPES.put(Enum.class, "solr.EnumFieldType");
-      
+
       System.setProperty(NUMERIC_POINTS_SYSPROP, "true");
     }
-    for (Map.Entry<Class<?>,String> entry : RANDOMIZED_NUMERIC_FIELDTYPES.entrySet()) {
-      System.setProperty("solr.tests." + entry.getKey().getSimpleName() + "FieldType",
-                         entry.getValue());
-
+    for (Map.Entry<Class<?>, String> entry : RANDOMIZED_NUMERIC_FIELDTYPES.entrySet()) {
+      System.setProperty(
+          "solr.tests." + entry.getKey().getSimpleName() + "FieldType", entry.getValue());
     }
   }
 
-  public static DistributedUpdateProcessor createDistributedUpdateProcessor(SolrQueryRequest req, SolrQueryResponse rsp,
-                                                                            UpdateRequestProcessor next) {
-    if(h.getCoreContainer().isZooKeeperAware()) {
+  public static DistributedUpdateProcessor createDistributedUpdateProcessor(
+      SolrQueryRequest req, SolrQueryResponse rsp, UpdateRequestProcessor next) {
+    if (h.getCoreContainer().isZooKeeperAware()) {
       return new DistributedZkUpdateProcessor(req, rsp, next);
     }
     return new DistributedUpdateProcessor(req, rsp, next);
   }
-  
+
   /**
-   * Cleans up the randomized sysproperties and variables set by {@link #randomizeNumericTypesProperties}
+   * Cleans up the randomized sysproperties and variables set by {@link
+   * #randomizeNumericTypesProperties}

Review comment:
       Fix this

##########
File path: solr/test-framework/src/java/org/apache/solr/cloud/AbstractBasicDistributedZkTestBase.java
##########
@@ -162,7 +162,8 @@ public static void beforeBDZKTClass() {
 
   @Override
   protected boolean useTlogReplicas() {
-    return false; // TODO: tlog replicas makes commits take way to long due to what is likely a bug and it's TestInjection use
+    return false; // TODO: tlog replicas makes commits take way to long due to what is likely a bug
+    // and it's TestInjection use

Review comment:
       Fix this

##########
File path: solr/test-framework/src/java/org/apache/solr/BaseDistributedSearchTestCase.java
##########
@@ -222,14 +219,15 @@ protected BaseDistributedSearchTestCase() {
    */
   protected BaseDistributedSearchTestCase(final String context) {
     this.context = context;
-    this.deadServers = new String[] {DEAD_HOST_1 + context,
-                                     DEAD_HOST_2 + context,
-                                     DEAD_HOST_3 + context};
+    this.deadServers =
+        new String[] {DEAD_HOST_1 + context, DEAD_HOST_2 + context, DEAD_HOST_3 + context};
   }
 
-  private final static int DEFAULT_MAX_SHARD_COUNT = 3;
+  private static final int DEFAULT_MAX_SHARD_COUNT = 3;
+
+  private int shardCount =
+      -1; // the actual number of solr cores that will be created in the cluster
 
-  private int shardCount = -1;      // the actual number of solr cores that will be created in the cluster

Review comment:
       Fix this




-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org