You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by John Burwell <jb...@basho.com> on 2013/07/01 17:04:16 UTC
Re: Review Request 11942: ProcessUtil cleanup
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11942/#review22599
-----------------------------------------------------------
utils/test/com/cloud/utils/ProcessUtilTest.java
<https://reviews.apache.org/r/11942/#comment46302>
Is there no system state on which to assert? What would cause this test to fail?
utils/test/com/cloud/utils/ProcessUtilTest.java
<https://reviews.apache.org/r/11942/#comment46303>
Is there no system state on which to assert? What would cause this test to fail?
utils/test/com/cloud/utils/ProcessUtilTest.java
<https://reviews.apache.org/r/11942/#comment46304>
Is there no system state on which to assert? What would cause this test to fail?
- John Burwell
On June 29, 2013, 3:51 p.m., Laszlo Hornyak wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11942/
> -----------------------------------------------------------
>
> (Updated June 29, 2013, 3:51 p.m.)
>
>
> Review request for cloudstack.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> - possible resource leak closed
> - file content read uses now commons-lang FileUtils
> - Added unit tests
>
>
> Diffs
> -----
>
> utils/src/com/cloud/utils/ProcessUtil.java c9fdf35
> utils/test/com/cloud/utils/ProcessUtilTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/11942/diff/
>
>
> Testing
> -------
>
> test included
>
>
> Thanks,
>
> Laszlo Hornyak
>
>
Re: Review Request 11942: ProcessUtil cleanup
Posted by Laszlo Hornyak <la...@gmail.com>.
> On July 1, 2013, 3:04 p.m., John Burwell wrote:
> > utils/test/com/cloud/utils/ProcessUtilTest.java, line 34
> > <https://reviews.apache.org/r/11942/diff/2/?file=313280#file313280line34>
> >
> > Is there no system state on which to assert? What would cause this test to fail?
This test verifies that the method with such arguments fails, that's why it expects an exception.
> On July 1, 2013, 3:04 p.m., John Burwell wrote:
> > utils/test/com/cloud/utils/ProcessUtilTest.java, line 40
> > <https://reviews.apache.org/r/11942/diff/2/?file=313280#file313280line40>
> >
> > Is there no system state on which to assert? What would cause this test to fail?
This test verifies that the method with such arguments fails, that's why it expects an exception.
> On July 1, 2013, 3:04 p.m., John Burwell wrote:
> > utils/test/com/cloud/utils/ProcessUtilTest.java, line 28
> > <https://reviews.apache.org/r/11942/diff/2/?file=313280#file313280line28>
> >
> > Is there no system state on which to assert? What would cause this test to fail?
Actually here I can add an assertion that the pid of this java process is written to the pid file, but I won't really be able to verify the pid in a platform independent way, since Java does not give an API to find out the PID of the current process. I can only use the very same method that the tested code is using, therefore an assume(is_os_linux) is also needed here.
The original purpose was to remove the minor resource leak from the original code (line 71) but I did not want to add a test that calls the method a lots of times.
- Laszlo
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11942/#review22599
-----------------------------------------------------------
On June 29, 2013, 3:51 p.m., Laszlo Hornyak wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11942/
> -----------------------------------------------------------
>
> (Updated June 29, 2013, 3:51 p.m.)
>
>
> Review request for cloudstack.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> - possible resource leak closed
> - file content read uses now commons-lang FileUtils
> - Added unit tests
>
>
> Diffs
> -----
>
> utils/src/com/cloud/utils/ProcessUtil.java c9fdf35
> utils/test/com/cloud/utils/ProcessUtilTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/11942/diff/
>
>
> Testing
> -------
>
> test included
>
>
> Thanks,
>
> Laszlo Hornyak
>
>