[GitHub] geode issue #687: GEODE-3258: Refactoring DiskStoreCommands
Github user pdxrunner commented on the issue: https://github.com/apache/geode/pull/687 I'll go ahead and review your command refactorings in this PR, keeping in mind that the test will be dealt with under the other JIRA. I agree with not changing those two DUnit tests at this time. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #687: GEODE-3258: Refactoring DiskStoreCommands
Github user YehEmily commented on the issue: https://github.com/apache/geode/pull/687 @pdxrunner The tests that cover these commands are part of a separate JIRA ticket ([GEODE-1359](https://issues.apache.org/jira/browse/GEODE-1359)), so @jinmeiliao recommended just refactoring the commands and leaving the tests to be refactored in the future. I think I will try to add tests to cover the rest of the commands to `DiskStoreCommandsJUnitTest`, since there are very few commands covered by that test, but I think I'll leave `DiskStoreCommandsDUnitTest` and `ListAndDescribeDiskStoreCommandsDUnitTest ` alone for now. What do you think? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #687: GEODE-3258: Refactoring DiskStoreCommands
Github user pdxrunner commented on the issue: https://github.com/apache/geode/pull/687 With regards to your comments on the test classes and possible duplication: I would consider splitting the existing test classes so that each of the new *Command classes has it's own tests. This may actually mean creating two test classes for each command - a Unit test to cover the functionality that doesn't need the overhead of the DUnit framework, and a DUnit test to cover what can't be accomplished in the Unit test. I would also consider making this refactoring incrementally, with separate pull requests for each command that is extracted from the original DiskStoreCommands class. The incremtnal PR's would then entail creating a Command class and test classes as needed. As a starter, you may want to create the ...Utils class when you first extract a command that needs the methods. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
