epugh commented on code in PR #3347:
URL: https://github.com/apache/solr/pull/3347#discussion_r2093331439
##########
solr/core/src/test/org/apache/solr/cli/TestSolrCLIRunExample.java:
##########
@@ -325,6 +330,99 @@ public void testFilmsExample() throws Exception {
testExample("films");
}
+ @Test
+ public void testAdditionalArgs() throws Exception {
+ String testStandaloneMode = LuceneTestCase.rarely() ? "--user-managed" :
"";
+ Path defaultSolrHomeDir = ExternalPaths.SERVER_HOME;
+ if (!Files.isDirectory(defaultSolrHomeDir)) {
Review Comment:
I don't know if this is required... I believe that all our tests that use
`ExternalPaths.SERVER_HOME` assume it's valid.. Altenratively, I think there
is a assert or assume type operation that only runs the test when a criteria is
met for things that ARE variable!
##########
solr/core/src/test/org/apache/solr/cli/TestSolrCLIRunExample.java:
##########
@@ -325,6 +330,99 @@ public void testFilmsExample() throws Exception {
testExample("films");
}
+ @Test
+ public void testAdditionalArgs() throws Exception {
+ String testStandaloneMode = LuceneTestCase.rarely() ? "--user-managed" :
"";
+ Path defaultSolrHomeDir = ExternalPaths.SERVER_HOME;
+ if (!Files.isDirectory(defaultSolrHomeDir)) {
+ fail(defaultSolrHomeDir + " not found and is required to run this
test!");
+ }
+
+ Path testSolrHomeDir = createTempDir();
+ Path solrServerDir = defaultSolrHomeDir.getParent();
+
+ // need a port to start the example server on
+ int bindPort = -1;
+ try (ServerSocket socket = new ServerSocket(0)) {
+ bindPort = socket.getLocalPort();
+ }
+
+ log.info("Selected port {} to start techproducts example Solr instance on
...", bindPort);
+
+ String sysProp = "solr.customprop.greet";
+ String[] toolArgs =
+ new String[] {
+ "-e",
+ "techproducts",
+ "--server-dir",
+ solrServerDir.toString(),
+ "--solr-home",
+ testSolrHomeDir.toString(),
+ "-p",
+ String.valueOf(bindPort),
+ testStandaloneMode,
+ "--jvm-opts",
+ "-D" + sysProp + "=hello"
+ };
+
+ // capture tool output to stdout
+ CLITestHelper.TestingRuntime runtime = new
CLITestHelper.TestingRuntime(true);
+
+ RunExampleExecutor executor = new RunExampleExecutor();
+ closeables.add(executor);
+
+ RunExampleTool tool = new RunExampleTool(executor, System.in, runtime);
+ try {
+ int status = tool.runTool(SolrCLI.processCommandLineArgs(tool,
toolArgs));
+
+ if (status == -1) {
+ // maybe it's the port, try again
+ try (ServerSocket socket = new ServerSocket(0)) {
+ bindPort = socket.getLocalPort();
+ }
+ Thread.sleep(100);
+ status = tool.runTool(SolrCLI.processCommandLineArgs(tool, toolArgs));
+ }
+
+ assertEquals("it should be ok " + tool + " " +
Arrays.toString(toolArgs), 0, status);
+
+ } catch (Exception e) {
+ log.error("RunExampleTool failed due to: ", e); // nowarn
+ throw e;
+ }
+
+ String propValue = null;
+ try (SolrClient solrClient =
+ new Http2SolrClient.Builder("http://localhost:"; + bindPort +
"/solr").build()) {
+ propValue = getSystemProperty(solrClient, sysProp);
+ }
+ assertEquals("System property provided with --jvm-opts not found",
"hello", propValue);
+ // stop the test instance
+ executor.execute(org.apache.commons.exec.CommandLine.parse("bin/solr stop
-p " + bindPort));
+ }
+
+ public String getSystemProperty(SolrClient solrClient, String propertyName)
throws Exception {
Review Comment:
I expected this to call the java system propertly method, or be a utility
method that we already have in Solr! On reading it, maybe a name like
"assertSolrRunningWithSystemProperty" would make it clearer?
##########
solr/core/src/test/org/apache/solr/cli/TestSolrCLIRunExample.java:
##########
@@ -325,6 +330,99 @@ public void testFilmsExample() throws Exception {
testExample("films");
}
+ @Test
+ public void testAdditionalArgs() throws Exception {
+ String testStandaloneMode = LuceneTestCase.rarely() ? "--user-managed" :
"";
+ Path defaultSolrHomeDir = ExternalPaths.SERVER_HOME;
+ if (!Files.isDirectory(defaultSolrHomeDir)) {
+ fail(defaultSolrHomeDir + " not found and is required to run this
test!");
+ }
+
+ Path testSolrHomeDir = createTempDir();
+ Path solrServerDir = defaultSolrHomeDir.getParent();
+
+ // need a port to start the example server on
+ int bindPort = -1;
+ try (ServerSocket socket = new ServerSocket(0)) {
+ bindPort = socket.getLocalPort();
+ }
+
+ log.info("Selected port {} to start techproducts example Solr instance on
...", bindPort);
+
+ String sysProp = "solr.customprop.greet";
+ String[] toolArgs =
+ new String[] {
+ "-e",
+ "techproducts",
+ "--server-dir",
+ solrServerDir.toString(),
+ "--solr-home",
+ testSolrHomeDir.toString(),
+ "-p",
+ String.valueOf(bindPort),
+ testStandaloneMode,
+ "--jvm-opts",
+ "-D" + sysProp + "=hello"
+ };
+
+ // capture tool output to stdout
+ CLITestHelper.TestingRuntime runtime = new
CLITestHelper.TestingRuntime(true);
+
+ RunExampleExecutor executor = new RunExampleExecutor();
+ closeables.add(executor);
+
+ RunExampleTool tool = new RunExampleTool(executor, System.in, runtime);
Review Comment:
I wonder if a helper method would reduice duplication between methods around
the running othe tool and hanlding of the sockets etc?
--
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-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
