rahulgoswami commented on code in PR #3378:
URL: https://github.com/apache/solr/pull/3378#discussion_r2114054937
##########
solr/core/src/test/org/apache/solr/cli/TestSolrCLIRunExample.java:
##########
@@ -102,7 +102,7 @@ public int execute(org.apache.commons.exec.CommandLine cmd)
throws IOException {
commandsExecuted.add(cmd);
String exe = cmd.getExecutable();
- if (exe.endsWith("solr")) {
+ if (exe.endsWith("solr") || exe.endsWith("solr.cmd")) {
Review Comment:
I agree. An assert is better. I'll chop off the "else".
Quick background: This change only impacts ***tests*** on Windows. Post the
fix for jvm-opts, command line execution runs fine.
The start flow via solr.cmd passes a "--script" parameter (which our tests
don't) and uses a different executor inside RunExampleTool from what the tests
use (RunExampleExecutor). Prior to recently merged fix for jvm-opts, because of
these reasons, the tests on Windows would also try to prepare a command line
with bin/solr (instead of bin/solr.cmd). Hence those tests would pass getting
into the "if" block in this PR, although in an unintended way.
So basically by fixing the code to do the right thing, the tests on Windows
are failing ;)
--
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
