You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by manuiiit <gi...@git.apache.org> on 2015/07/22 22:34:26 UTC
[GitHub] cloudstack pull request: Added a null value exception in LibvirtBa...
GitHub user manuiiit opened a pull request:
https://github.com/apache/cloudstack/pull/617
Added a null value exception in LibvirtBackupSnapshotCommandWrapper.java
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/manuiiit/cloudstack master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/617.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #617
----
commit 66d7ed3f1a9d06bbdf388cbb7b7e41e945b0a66d
Author: Maneesha P <ma...@citrix.com>
Date: 2015-07-22T19:15:00Z
Added a null value exception
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Added a null value exception in LibvirtBa...
Posted by manuiiit <gi...@git.apache.org>.
Github user manuiiit commented on the pull request:
https://github.com/apache/cloudstack/pull/617#issuecomment-123998699
Hi Wilder,
The LibvirtBackupSnapshotCommandWrapper class has no unit
tests.Can I add test cases that covers the exception.
Regards,
P.Maneesha.
On Thu, Jul 23, 2015 at 11:35 AM, Wilder Rodrigues <notifications@github.com
> wrote:
> Hi @manuiiit <https://github.com/manuiiit>
>
> I checked the LibvirtBackupSnapshotCommandWrapper class and its coverage,
> which is still poor compared to the other classes in the KVM hypervisor
> plugin.
>
> Please, proceed with the changes @karuturi <https://github.com/karuturi>
> suggested and I will increase the tests on the class in a separate PR.
>
> It is currently 6.9% covered, which is very bad.
>
> Cheers,
> Wilder
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/cloudstack/pull/617#issuecomment-123987028>.
>
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Added a null value exception in LibvirtBa...
Posted by wilderrodrigues <gi...@git.apache.org>.
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/617#issuecomment-124001316
Hi Maneesha,
Yep, it has... I wrote them.
All the wrappers classes have in total 145 unit tests. Have a look at LibvirtComputingResourceTes, i think that's the name of the class.
As I said, due to complexity of the code that one test is poor, but most of the wrapper classes have 100% coverage.
No worries, I'm busy with improving a couple of tests and will also do the one for the backup snapshot.
Apply the change Rajani suggested so we can get it merged.
Cheers,
Wilder
Sent from my iPhone
On 23 Jul 2015, at 08:55, Maneesha P <no...@github.com>> wrote:
Hi Wilder,
The LibvirtBackupSnapshotCommandWrapper class has no unit
tests.Can I add test cases that covers the exception.
Regards,
P.Maneesha.
On Thu, Jul 23, 2015 at 11:35 AM, Wilder Rodrigues <no...@github.com>
> wrote:
> Hi @manuiiit <https://github.com/manuiiit>
>
> I checked the LibvirtBackupSnapshotCommandWrapper class and its coverage,
> which is still poor compared to the other classes in the KVM hypervisor
> plugin.
>
> Please, proceed with the changes @karuturi <https://github.com/karuturi>
> suggested and I will increase the tests on the class in a separate PR.
>
> It is currently 6.9% covered, which is very bad.
>
> Cheers,
> Wilder
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/cloudstack/pull/617#issuecomment-123987028>.
>
—
Reply to this email directly or view it on GitHub<https://github.com/apache/cloudstack/pull/617#issuecomment-123998699>.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Added a null value exception in LibvirtBa...
Posted by wilderrodrigues <gi...@git.apache.org>.
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/617#issuecomment-123879270
You added code to a class that was recently refactored and was covered by unit tests. Please add a new unit test to cover the new flow.
Cheers,
Wilder
Sent from my iPhone
On 22 Jul 2015, at 22:34, manuiiit <no...@github.com>> wrote:
________________________________
You can view, comment on, or merge this pull request online at:
https://github.com/apache/cloudstack/pull/617
Commit Summary
* Added a null value exception
File Changes
* M plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtBackupSnapshotCommandWrapper.java<https://github.com/apache/cloudstack/pull/617/files#diff-0> (9)
Patch Links:
* https://github.com/apache/cloudstack/pull/617.patch
* https://github.com/apache/cloudstack/pull/617.diff
—
Reply to this email directly or view it on GitHub<https://github.com/apache/cloudstack/pull/617>.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Added a null value exception in LibvirtBa...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cloudstack/pull/617
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Added a null value exception in LibvirtBa...
Posted by wilderrodrigues <gi...@git.apache.org>.
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/617#issuecomment-123987028
Hi @manuiiit
I checked the LibvirtBackupSnapshotCommandWrapper class and its coverage, which is still poor compared to the other classes in the KVM hypervisor plugin.
Please, proceed with the changes @karuturi suggested and I will increase the tests on the class in a separate PR.
It is currently 6.9% covered, which is very bad.
Cheers,
Wilder
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Added a null value exception in LibvirtBa...
Posted by karuturi <gi...@git.apache.org>.
Github user karuturi commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/617#discussion_r35289154
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtBackupSnapshotCommandWrapper.java ---
@@ -25,6 +25,8 @@
import java.io.IOException;
import java.text.MessageFormat;
+import com.thoughtworks.xstream.mapper.Mapper;
+import org.apache.commons.lang.ObjectUtils;
--- End diff --
remove the unused imports and format the code as per coding conventions.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Added a null value exception in LibvirtBa...
Posted by karuturi <gi...@git.apache.org>.
Github user karuturi commented on the pull request:
https://github.com/apache/cloudstack/pull/617#issuecomment-124044971
merging to master now
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---