You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Miguel Ferreira <mi...@me.com> on 2014/02/21 17:43:48 UTC
Review Request 18358: NetUtils unit testing
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18358/
-----------------------------------------------------------
Review request for cloudstack, daan Hoogland and Hugo Trippaers.
Repository: cloudstack-git
Description
-------
- Refactor tests:
- Upgrade tests to use jUnit4
- Break big tests in small unit tests
- Replace assertTrue/False with complex conditions by assertThat with
specific matchers
- Remove dead code:
- Private static method never called locally
- Add test for method that validates CIDRs
Diffs
-----
utils/src/com/cloud/utils/net/NetUtils.java c22e39a
utils/test/com/cloud/utils/net/NetUtilsTest.java d3e283c
Diff: https://reviews.apache.org/r/18358/diff/
Testing
-------
Ran all the tests in the test class before and after refactoring.
Thanks,
Miguel Ferreira
Re: Review Request 18358: NetUtils unit testing
Posted by Miguel Ferreira <mi...@me.com>.
> On Feb. 21, 2014, 6:54 p.m., Laszlo Hornyak wrote:
> > Hi Miguel,
> >
> > I started working on verification and I ran into some dependency problem. hamcrest-library is needed as dependency and junit 4.10 needs to be upgraded to 4.11
> >
> >
Hi Laszio,
Good catch! I did not run the tets from maven, only from Eclipse and there I had no dependency problem.
I applied your patch on my code base and it fixes the errors for me.
In addition to that I've added a variable for the version of hamcrest, just like it is done for the other dependencies.
I will update the patch with my changes and yours together. Please let me know if that is not the best way to go and I'll make a new patch with just my changes.
- Miguel
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18358/#review35179
-----------------------------------------------------------
On Feb. 21, 2014, 4:43 p.m., Miguel Ferreira wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18358/
> -----------------------------------------------------------
>
> (Updated Feb. 21, 2014, 4:43 p.m.)
>
>
> Review request for cloudstack, daan Hoogland and Hugo Trippaers.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> - Refactor tests:
> - Upgrade tests to use jUnit4
> - Break big tests in small unit tests
> - Replace assertTrue/False with complex conditions by assertThat with
> specific matchers
> - Remove dead code:
> - Private static method never called locally
> - Add test for method that validates CIDRs
>
>
> Diffs
> -----
>
> utils/src/com/cloud/utils/net/NetUtils.java c22e39a
> utils/test/com/cloud/utils/net/NetUtilsTest.java d3e283c
>
> Diff: https://reviews.apache.org/r/18358/diff/
>
>
> Testing
> -------
>
> Ran all the tests in the test class before and after refactoring.
>
>
> Thanks,
>
> Miguel Ferreira
>
>
Re: Review Request 18358: NetUtils unit testing
Posted by Laszlo Hornyak <la...@gmail.com>.
Miguel, here is my patch that for me fixed the dependency problems. Other
than that, it looks good.
Thank you,
Laszlo
On Fri, Feb 21, 2014 at 7:54 PM, Laszlo Hornyak <la...@gmail.com>wrote:
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18358/
>
> Hi Miguel,
>
> I started working on verification and I ran into some dependency problem. hamcrest-library is needed as dependency and junit 4.10 needs to be upgraded to 4.11
>
>
>
> - Laszlo Hornyak
>
> On February 21st, 2014, 4:43 p.m. UTC, Miguel Ferreira wrote:
> Review request for cloudstack, daan Hoogland and Hugo Trippaers.
> By Miguel Ferreira.
>
> *Updated Feb. 21, 2014, 4:43 p.m.*
> *Repository: * cloudstack-git
> Description
>
> - Refactor tests:
> - Upgrade tests to use jUnit4
> - Break big tests in small unit tests
> - Replace assertTrue/False with complex conditions by assertThat with
> specific matchers
> - Remove dead code:
> - Private static method never called locally
> - Add test for method that validates CIDRs
>
> Testing
>
> Ran all the tests in the test class before and after refactoring.
>
> Diffs
>
> - utils/src/com/cloud/utils/net/NetUtils.java (c22e39a)
> - utils/test/com/cloud/utils/net/NetUtilsTest.java (d3e283c)
>
> View Diff <https://reviews.apache.org/r/18358/diff/>
>
--
EOF
Re: Review Request 18358: NetUtils unit testing
Posted by Laszlo Hornyak <la...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18358/#review35179
-----------------------------------------------------------
Hi Miguel,
I started working on verification and I ran into some dependency problem. hamcrest-library is needed as dependency and junit 4.10 needs to be upgraded to 4.11
- Laszlo Hornyak
On Feb. 21, 2014, 4:43 p.m., Miguel Ferreira wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18358/
> -----------------------------------------------------------
>
> (Updated Feb. 21, 2014, 4:43 p.m.)
>
>
> Review request for cloudstack, daan Hoogland and Hugo Trippaers.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> - Refactor tests:
> - Upgrade tests to use jUnit4
> - Break big tests in small unit tests
> - Replace assertTrue/False with complex conditions by assertThat with
> specific matchers
> - Remove dead code:
> - Private static method never called locally
> - Add test for method that validates CIDRs
>
>
> Diffs
> -----
>
> utils/src/com/cloud/utils/net/NetUtils.java c22e39a
> utils/test/com/cloud/utils/net/NetUtilsTest.java d3e283c
>
> Diff: https://reviews.apache.org/r/18358/diff/
>
>
> Testing
> -------
>
> Ran all the tests in the test class before and after refactoring.
>
>
> Thanks,
>
> Miguel Ferreira
>
>
Re: Review Request 18358: NetUtils unit testing
Posted by daan Hoogland <da...@gmail.com>.
> On Feb. 24, 2014, 8:11 p.m., Laszlo Hornyak wrote:
> > Verified, looks good. Daan, ok to merge?
yes, looks ok
- daan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18358/#review35319
-----------------------------------------------------------
On Feb. 22, 2014, 1:41 p.m., Miguel Ferreira wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18358/
> -----------------------------------------------------------
>
> (Updated Feb. 22, 2014, 1:41 p.m.)
>
>
> Review request for cloudstack, daan Hoogland and Hugo Trippaers.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> - Refactor tests:
> - Upgrade tests to use jUnit4
> - Break big tests in small unit tests
> - Replace assertTrue/False with complex conditions by assertThat with
> specific matchers
> - Remove dead code:
> - Private static method never called locally
> - Add test for method that validates CIDRs
>
>
> Diffs
> -----
>
> pom.xml 1e9e8d8
> utils/src/com/cloud/utils/net/NetUtils.java c22e39a
> utils/test/com/cloud/utils/net/NetUtilsTest.java d3e283c
>
> Diff: https://reviews.apache.org/r/18358/diff/
>
>
> Testing
> -------
>
> Ran all the tests in the test class before and after refactoring.
>
>
> Thanks,
>
> Miguel Ferreira
>
>
Re: Review Request 18358: NetUtils unit testing
Posted by Laszlo Hornyak <la...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18358/#review35319
-----------------------------------------------------------
Verified, looks good. Daan, ok to merge?
- Laszlo Hornyak
On Feb. 22, 2014, 1:41 p.m., Miguel Ferreira wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18358/
> -----------------------------------------------------------
>
> (Updated Feb. 22, 2014, 1:41 p.m.)
>
>
> Review request for cloudstack, daan Hoogland and Hugo Trippaers.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> - Refactor tests:
> - Upgrade tests to use jUnit4
> - Break big tests in small unit tests
> - Replace assertTrue/False with complex conditions by assertThat with
> specific matchers
> - Remove dead code:
> - Private static method never called locally
> - Add test for method that validates CIDRs
>
>
> Diffs
> -----
>
> pom.xml 1e9e8d8
> utils/src/com/cloud/utils/net/NetUtils.java c22e39a
> utils/test/com/cloud/utils/net/NetUtilsTest.java d3e283c
>
> Diff: https://reviews.apache.org/r/18358/diff/
>
>
> Testing
> -------
>
> Ran all the tests in the test class before and after refactoring.
>
>
> Thanks,
>
> Miguel Ferreira
>
>
Re: Review Request 18358: NetUtils unit testing
Posted by Laszlo Hornyak <la...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18358/#review35430
-----------------------------------------------------------
Ship it!
Ship It!
- Laszlo Hornyak
On Feb. 22, 2014, 1:41 p.m., Miguel Ferreira wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18358/
> -----------------------------------------------------------
>
> (Updated Feb. 22, 2014, 1:41 p.m.)
>
>
> Review request for cloudstack, daan Hoogland and Hugo Trippaers.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> - Refactor tests:
> - Upgrade tests to use jUnit4
> - Break big tests in small unit tests
> - Replace assertTrue/False with complex conditions by assertThat with
> specific matchers
> - Remove dead code:
> - Private static method never called locally
> - Add test for method that validates CIDRs
>
>
> Diffs
> -----
>
> pom.xml 1e9e8d8
> utils/src/com/cloud/utils/net/NetUtils.java c22e39a
> utils/test/com/cloud/utils/net/NetUtilsTest.java d3e283c
>
> Diff: https://reviews.apache.org/r/18358/diff/
>
>
> Testing
> -------
>
> Ran all the tests in the test class before and after refactoring.
>
>
> Thanks,
>
> Miguel Ferreira
>
>
Re: Review Request 18358: NetUtils unit testing
Posted by Miguel Ferreira <mi...@me.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18358/
-----------------------------------------------------------
(Updated Feb. 22, 2014, 1:41 p.m.)
Review request for cloudstack, daan Hoogland and Hugo Trippaers.
Changes
-------
Add hamcrest dependency to pom (contribution of Laszio Hornyak)
Repository: cloudstack-git
Description
-------
- Refactor tests:
- Upgrade tests to use jUnit4
- Break big tests in small unit tests
- Replace assertTrue/False with complex conditions by assertThat with
specific matchers
- Remove dead code:
- Private static method never called locally
- Add test for method that validates CIDRs
Diffs (updated)
-----
pom.xml 1e9e8d8
utils/src/com/cloud/utils/net/NetUtils.java c22e39a
utils/test/com/cloud/utils/net/NetUtilsTest.java d3e283c
Diff: https://reviews.apache.org/r/18358/diff/
Testing
-------
Ran all the tests in the test class before and after refactoring.
Thanks,
Miguel Ferreira