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 thing? ---------------------------------------------------------------- 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