daniellavoie commented on a change in pull request #5674:
URL: https://github.com/apache/incubator-pinot/pull/5674#discussion_r452324220



##########
File path: 
pinot-spi/src/test/java/org/apache/pinot/spi/env/PinotConfigurationTest.java
##########
@@ -24,24 +24,116 @@
 import java.nio.file.Files;
 import java.nio.file.Paths;
 import java.nio.file.StandardCopyOption;
+import java.util.Arrays;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 
+import org.apache.commons.configuration.ConfigurationException;
+import org.apache.commons.configuration.PropertiesConfiguration;
+import org.apache.pinot.spi.ingestion.batch.spec.PinotFSSpec;
 import org.testng.Assert;
 import org.testng.annotations.Test;
 
 
 public class PinotConfigurationTest {
+
+  @Test
+  public void assertBaseOperations() {

Review comment:
       Pleasure is mine @mayankshriv, 
   
   Line 116 of `PinotConfiguration` executes some tests on config subsettings. 
Kishore added tests specifically for PinotFS, any additional contextual tests 
related to Crypter and Server should be hosted in their dedicated modules. That 
would be a great addition but maybe should be tied to a future PR that aimed at 
adding tests for these specific modules. What do you think?




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

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org

Reply via email to