Github user rafaelweingartner commented on the pull request: https://github.com/apache/cloudstack/pull/700#issuecomment-132197866 Hi all, Sorry the delay. Well, @anshul1886 IMHO the test cases add value to the code, they are making sure that those paths are properly coded (that may be too simple; however in such a huge, complex and complicated code such as the ACS they are needed). We try to use TDD to develop anything we are doing here. Therefore, when we were making those changes we did the following: ⢠Created an abstract method: com.cloud.hypervisor.xenserver.resource.CitrixResourceBase.getPatchFilePath() ⢠Then we just generated that method in CitrixResourceBase subclasses ⢠Extracted the body of the method âgetPatchFilesâ from CitrixResourceBase subclasses ⢠At that moment we already had the methods we needed, so we could start creating the tests to check if each instance was returning the expected path. Every single test was failing at this moment as we expected. ⢠Then we started coding the methods in each class that was needed. After that we ran the tests to check if everything was ok. I believe those tests add value and that they are not a burden as pointed out by @DaanHoogland , if some future commit comes to touch that code and change the paths by mistake our test cases will get that. Additionally, I also believe that tests cases should be as simple as that, if the test method is too complicated there is something wrong either with the code or with the coder. But at the end if you guys think that they are not necessary, I can remove them. No hard feelings ;)
--- 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. ---