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 [email protected] or file a JIRA ticket
with INFRA.
---
