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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
