epugh commented on code in PR #5204:
URL: https://github.com/apache/solr/pull/5204#discussion_r4358568411
##########
solr/packaging/test/test_basic_auth.bats:
##########
@@ -25,7 +25,7 @@ setup() {
# The auth command exports some system variables that are injected as basic
auth username and password,
# however that defeats our test so fake that out via --solr-include-file
param specifing a bogus path.
- solr auth enable --type basicAuth --credentials name:password
--solr-include-file /force/credentials/to/be/supplied
+ SOLR_INCLUDE=/force/credentials/to/be/supplied solr auth enable --type
basicAuth --credentials name:password
Review Comment:
if the way to pass this setting in is via a system property, that maybe begs
teh question why do we have the parameter at all?
##########
solr/packaging/test/test_help.bats:
##########
@@ -23,38 +23,39 @@ setup() {
@test "solr help flag prints help" {
run -1 solr --help
- assert_output --partial 'Usage: solr COMMAND OPTIONS'
Review Comment:
i know we are doing "the right thing", but a part of me just wishes we
removed the commons-cli testing at this point... but this is "the right
way"...
##########
solr/solr-ref-guide/modules/deployment-guide/pages/cli/solr-auth.adoc:
##########
@@ -66,9 +66,6 @@ Updates or enables/disables authentication. Must be run on
the Solr server itsel
*-s*, *--solr-connection*=_<solrConnection>_::
Zookeeper or HTTP(s) connection string; unnecessary if SOLR_CONNECTION is
defined in solr.in.sh; otherwise, defaults to localhost:9983.
-*--solr-include-file*=_FILE_::
Review Comment:
why can't we just keep this? Or is this just a property that no one uses,
so we simplifeid by making it a system property?
##########
solr/packaging/test/bats_helper.bash:
##########
@@ -115,3 +115,13 @@ wait_for() {
return 1 # Timeout reached
}
+
+# Utility function for asserting between the commons-cli wording ($1), or,
when running against the picocli CLI, the picocli wording ($2)
+# Todo: Remove this once commons-cli cleanup happens
Review Comment:
i think in java code it's "TODO", do we actually have any reports that
highlihgt todo code items?
--
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]
