You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@commons.apache.org by TheRealHaui <gi...@git.apache.org> on 2017/07/10 18:44:03 UTC

[GitHub] commons-numbers pull request #8: Add-some-Unit-Tests

GitHub user TheRealHaui opened a pull request:

    https://github.com/apache/commons-numbers/pull/8

    Add-some-Unit-Tests

    I added some Unit Tests to increase code coverage which I want to contribute.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/TheRealHaui/commons-numbers master

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/commons-numbers/pull/8.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 #8
    
----
commit eb962ef6f5348a706cec17e33fa05c2b17435cb8
Author: Michael Hausegger <ha...@googlemail.com>
Date:   2017-07-10T18:33:03Z

    Add-some-Unit-Tests Add some Unit Tests to increase code coverage.

----


---
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] commons-numbers pull request #8: Add-some-Unit-Tests

Posted by ameyjadiye <gi...@git.apache.org>.
Github user ameyjadiye commented on a diff in the pull request:

    https://github.com/apache/commons-numbers/pull/8#discussion_r126515495
  
    --- Diff: commons-numbers-combinatorics/src/test/java/org/apache/commons/numbers/combinatorics/LogFactorialTest.java ---
    @@ -115,4 +116,20 @@ private double logFactorial(final int n) {
             }
             return logSum;
         }
    +
    +    @Test
    +    public void testValueThrowsCombinatoricsException() {
    +
    --- End diff --
    
    Same here, no empty line


---
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] commons-numbers issue #8: Add-some-Unit-Tests

Posted by TheRealHaui <gi...@git.apache.org>.
Github user TheRealHaui commented on the issue:

    https://github.com/apache/commons-numbers/pull/8
  
    You're right, that's the most simple way.
    Pushed changes now to master.
    They appear here in this pull request.
    Furthermore I will close pull request #9.


---
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] commons-numbers pull request #8: Add-some-Unit-Tests

Posted by ameyjadiye <gi...@git.apache.org>.
Github user ameyjadiye commented on a diff in the pull request:

    https://github.com/apache/commons-numbers/pull/8#discussion_r126515101
  
    --- Diff: commons-numbers-arrays/src/test/java/org/apache/commons/numbers/arrays/SafeNormTest.java ---
    @@ -24,39 +26,52 @@
         @Test
         public void testTiny() {
             final double s = 1e-320;
    -        final double[] v = new double[] { s, s };
    -        Assert.assertEquals(Math.sqrt(2) * s, SafeNorm.value(v), 0d);
    +        final double[] v = new double[]{s, s};
    +        assertEquals(Math.sqrt(2) * s, SafeNorm.value(v), 0d);
         }
     
         @Test
         public void testBig() {
             final double s = 1e300;
    -        final double[] v = new double[] { s, s };
    -        Assert.assertEquals(Math.sqrt(2) * s, SafeNorm.value(v), 0d);
    +        final double[] v = new double[]{s, s};
    +        assertEquals(Math.sqrt(2) * s, SafeNorm.value(v), 0d);
         }
     
         @Test
         public void testOne3D() {
             final double s = 1;
    -        final double[] v = new double[] { s, s, s };
    -        Assert.assertEquals(Math.sqrt(3), SafeNorm.value(v), 0d);
    +        final double[] v = new double[]{s, s, s};
    +        assertEquals(Math.sqrt(3), SafeNorm.value(v), 0d);
         }
     
         @Test
         public void testUnit3D() {
    -        Assert.assertEquals(1, SafeNorm.value(new double[] { 1, 0, 0 }), 0d);
    -        Assert.assertEquals(1, SafeNorm.value(new double[] { 0, 1, 0 }), 0d);
    -        Assert.assertEquals(1, SafeNorm.value(new double[] { 0, 0, 1 }), 0d);
    +        assertEquals(1, SafeNorm.value(new double[]{1, 0, 0}), 0d);
    +        assertEquals(1, SafeNorm.value(new double[]{0, 1, 0}), 0d);
    +        assertEquals(1, SafeNorm.value(new double[]{0, 0, 1}), 0d);
         }
     
         @Test
         public void testSimple() {
    -        final double[] v = new double[] { -0.9, 8.7, -6.5, -4.3, -2.1, 0, 1.2, 3.4, -5.6, 7.8, 9.0 };
    +        final double[] v = new double[]{-0.9, 8.7, -6.5, -4.3, -2.1, 0, 1.2, 3.4, -5.6, 7.8, 9.0};
             double n = 0;
             for (int i = 0; i < v.length; i++) {
                 n += v[i] * v[i];
             }
             final double expected = Math.sqrt(n);
    -        Assert.assertEquals(expected, SafeNorm.value(v), 0d);
    +        assertEquals(expected, SafeNorm.value(v), 0d);
    +    }
    +
    +    @Test
    +    public void testValueReturningPositive() {
    +
    --- End diff --
    
    Say no to empty lines


---
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] commons-numbers issue #8: Add-some-Unit-Tests

Posted by ameyjadiye <gi...@git.apache.org>.
Github user ameyjadiye commented on the issue:

    https://github.com/apache/commons-numbers/pull/8
  
    Thanks @TheRealHaui. will take another round or review.
    
    If you wish that new code to be come under same PR you have to push to same branch :wink: 
    for #8 you pushed to ```TheRealHaui:master``` while for #9 you pushed to ```TheRealHaui:Add-some-Unit-Tests```.


---
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] commons-numbers issue #8: Add-some-Unit-Tests

Posted by coveralls <gi...@git.apache.org>.
Github user coveralls commented on the issue:

    https://github.com/apache/commons-numbers/pull/8
  
    
    [![Coverage Status](https://coveralls.io/builds/12328326/badge)](https://coveralls.io/builds/12328326)
    
    Coverage increased (+3.2%) to 86.303% when pulling **eb962ef6f5348a706cec17e33fa05c2b17435cb8 on TheRealHaui:master** into **72a0c9d499876c627d09d379330f35708884158e on apache:master**.



---
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] commons-numbers pull request #8: Add-some-Unit-Tests

Posted by ameyjadiye <gi...@git.apache.org>.
Github user ameyjadiye commented on a diff in the pull request:

    https://github.com/apache/commons-numbers/pull/8#discussion_r126514781
  
    --- Diff: commons-numbers-combinatorics/src/test/java/org/apache/commons/numbers/combinatorics/FactorialDoubleTest.java ---
    @@ -121,10 +124,31 @@ public void testCacheDecrease() {
          * Direct multiplication implementation.
          */
         private double factorialDirect(int n) {
    +
             double result = 1;
    +
             for (int i = 2; i <= n; i++) {
                 result *= i;
             }
    +
             return result;
    +
    +
         }
    +
    +  @Test
    +  public void testWithCacheThrowsCombinatoricsException() {
    +
    --- End diff --
    
    Don't keep empty lines. by guidelines after method name no empty line should be there neither before ending method.


---
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] commons-numbers issue #8: Add-some-Unit-Tests

Posted by TheRealHaui <gi...@git.apache.org>.
Github user TheRealHaui commented on the issue:

    https://github.com/apache/commons-numbers/pull/8
  
    @ameyjadiye 
    Removed evil empty lines in pull request #9.
    Don't know why Github didn't add the change to the original pull request.  
    Anyway, I think that doesn't bother a lot.


---
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] commons-numbers issue #8: Add-some-Unit-Tests

Posted by ameyjadiye <gi...@git.apache.org>.
Github user ameyjadiye commented on the issue:

    https://github.com/apache/commons-numbers/pull/8
  
    Yes, since you have opened it, you have right to close #9 , but simply push those changes to your master so those will be available in #8. Thanks.


---
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] commons-numbers issue #8: Add-some-Unit-Tests

Posted by TheRealHaui <gi...@git.apache.org>.
Github user TheRealHaui commented on the issue:

    https://github.com/apache/commons-numbers/pull/8
  
    Yes ...
    Forgot that I created a branch explicitly yesterday.
    After pughing ...
    Think that doesn't bother a lot, simply delete the pull request you do not need.


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