> 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 > >
