sijie commented on issue #1093: BP-27: New BookKeeper CLI URL: https://github.com/apache/bookkeeper/pull/1093#issuecomment-362342787 > Can I suggest that as you submit the changes, you submit each command individually, so that it's easier to review? you mean this PR in #1094 or you mean the changes in future? If you mean #1094 , the change there is mainly for setting up the framework and only 3 commands on each category is moved. so I think it is already a minimal change that I can get to for this review - you can get an overview on how the new CLI looks like. If you mean the changes in futures, yes, there will be and should be one command per PR. > Integration tests for these would also be good, since unit testing can't test the bash script part. You don't need to test bash script every command. The testing of bash script should mainly just focus on if it is taking the environment settings correctly, invoking the CLI program correctly. That means the tests should be: 1) bash script testing: focus on testing the script itself, if it is taking the environment settings correctly, invoking the CLI program correctly. And it should stop after testing it can invoke the program. 2) CLI program testing: focus on testing parsing and ensure the correct command is invoking and right parameters are passing into command. 3) Command logic testing: focus on testing the logic for individual commands. This BP covers both 2) and 3). while 1) is related but it is a different topic in general, we need to test all the scripts we shipped along with the release. so it will have to be a different task than the one here. Hope this make the clarification.
---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services
