Re: Review Request 18358: NetUtils unit testing

[Laszlo Hornyak]

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

[daan Hoogland]

> 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

[Laszlo Hornyak]

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

[Miguel Ferreira]

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

Re: Review Request 18358: NetUtils unit testing

[Miguel Ferreira]

> 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

[Laszlo Hornyak]

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



-- 

EOF
From 728239ddabef9b9775df74df47fd58a117e1515a Mon Sep 17 00:00:00 2001
From: Laszlo Hornyak 
Date: Fri, 21 Feb 2014 19:58:12 +0100
Subject: [PATCH 2/2] Fixes for 1c4bf5f163bd721daa0b53e3d43d3de13a3b05d9

- upgraded junit to 4.11
- added hamcrest-library 1.3 dependency, test are building on it

Signed-off-by: Laszlo Hornyak 
---
 pom.xml | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/pom.xml b/pom.xml
index 1e9e8d8..4532679 100644
--- a/pom.xml
+++ b/pom.xml
@@ -49,7 +49,8 @@
 1.1.1
 0.5
 3.0
-4.10
+
+4.11
 1.46
 0.1.42
 2.0.0
@@ -414,6 +415,12 @@
   test
 
 
+  org.hamcrest
+  hamcrest-library
+  1.3
+  test
+
+
   org.mockito
   mockito-all
   1.9.5
-- 
1.8.3.1

Re: Review Request 18358: NetUtils unit testing

[Laszlo Hornyak]

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

Review Request 18358: NetUtils unit testing

[Miguel Ferreira]

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