[GitHub] commons-compress pull request #33: add-some-Unit-Tests Added some Unit Tests...

classic Classic list List threaded Threaded
15 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] commons-compress pull request #33: add-some-Unit-Tests Added some Unit Tests...

ajs6f
GitHub user TheRealHaui opened a pull request:

    https://github.com/apache/commons-compress/pull/33

    add-some-Unit-Tests Added some Unit Tests to increase code coverage.

    I have added some Unit Tests to increase code coverage and want to contribute them.

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

    $ git pull https://github.com/TheRealHaui/commons-compress add-some-Unit-Tests

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

    https://github.com/apache/commons-compress/pull/33.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 #33
   
----
commit 2f45456527f2631ef8b4bb09aa0ad30afda02b5f
Author: Michael Hausegger <[hidden email]>
Date:   2017-06-13T21:47:50Z

    add-some-Unit-Tests Added 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 [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] commons-compress issue #33: add-some-Unit-Tests Added some Unit Tests to inc...

ajs6f
Github user TheRealHaui commented on the issue:

    https://github.com/apache/commons-compress/pull/33
 
    @bodewig
    Could you be so kind an review my contribution and - of course - most of all pull it into the code base.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] commons-compress issue #33: add-some-Unit-Tests Added some Unit Tests to inc...

ajs6f
In reply to this post by ajs6f
Github user coveralls commented on the issue:

    https://github.com/apache/commons-compress/pull/33
 
   
    [![Coverage Status](https://:/builds/11957412/badge)](https://:/builds/11957412)
   
    Coverage increased (+0.2%) to 84.524% when pulling **2f45456527f2631ef8b4bb09aa0ad30afda02b5f on TheRealHaui:add-some-Unit-Tests** into **0d8ab18c86f4edb38a73de1512483bb5b876bb1f 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 [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] commons-compress issue #33: add-some-Unit-Tests Added some Unit Tests to inc...

ajs6f
In reply to this post by ajs6f
Github user bodewig commented on the issue:

    https://github.com/apache/commons-compress/pull/33
 
    Fortunately I'm not the only one who could merge this :-)
   
    More seriously, it may take a bit of time until I get there, but I will.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] commons-compress issue #33: add-some-Unit-Tests Added some Unit Tests to inc...

ajs6f
In reply to this post by ajs6f
Github user bodewig commented on the issue:

    https://github.com/apache/commons-compress/pull/33
 
    Thanks @TheRealHaui
   
    I agree with your comment on `ChecksumCalculatingInputStreamTest#testGetValueThrowsNullPointerException` this looks like a bug. So I'd rather remove the test and see the bug fixed. Do you want to take a stab?
   
    Also, the Commons Compress community has decided to not use author tags, I'd have to strip yours before merging the PR. Could you please remove them yourself if you modify this PR? Feel free to add yourself as a contributor to the POM.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] commons-compress issue #33: add-some-Unit-Tests Added some Unit Tests to inc...

ajs6f
In reply to this post by ajs6f
Github user coveralls commented on the issue:

    https://github.com/apache/commons-compress/pull/33
 
   
    [![Coverage Status](https://:/builds/12007847/badge)](https://:/builds/12007847)
   
    Coverage increased (+0.5%) to 84.812% when pulling **96ca8ceeddbd20ec38b9211260b4b91107b0be2d on TheRealHaui:add-some-Unit-Tests** into **0d8ab18c86f4edb38a73de1512483bb5b876bb1f 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 [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] commons-compress issue #33: add-some-Unit-Tests Added some Unit Tests to inc...

ajs6f
In reply to this post by ajs6f
Github user coveralls commented on the issue:

    https://github.com/apache/commons-compress/pull/33
 
   
    [![Coverage Status](https://:/builds/12008341/badge)](https://:/builds/12008341)
   
    Coverage increased (+0.3%) to 84.673% when pulling **be0f11f4dbffb5ca1f903a6f07de744687d303c3 on TheRealHaui:add-some-Unit-Tests** into **0d8ab18c86f4edb38a73de1512483bb5b876bb1f 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 [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] commons-compress issue #33: add-some-Unit-Tests Added some Unit Tests to inc...

ajs6f
In reply to this post by ajs6f
Github user coveralls commented on the issue:

    https://github.com/apache/commons-compress/pull/33
 
   
    [![Coverage Status](https://:/builds/12008449/badge)](https://:/builds/12008449)
   
    Coverage increased (+0.3%) to 84.673% when pulling **3b46bb5dc3aeb3ca68062c10589e049c9eb8551d on TheRealHaui:add-some-Unit-Tests** into **0d8ab18c86f4edb38a73de1512483bb5b876bb1f 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 [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] commons-compress issue #33: add-some-Unit-Tests Added some Unit Tests to inc...

ajs6f
In reply to this post by ajs6f
Github user TheRealHaui commented on the issue:

    https://github.com/apache/commons-compress/pull/33
 
    @bodewig
    Thank you for your kind response!
    Really appreciate that!
   
    Therefore I've made all the changes you requested/proposed.
    And of course added myself as a contributor as heavily requested by you.
    Really couldn't disappoint you regarding that special topic. :-)
   
    Furthermore I've created a Jira Task for the bespoken bug: https://issues.apache.org/jira/browse/COMPRESS-412
    And fixed it in an own branch which I've commited, created tests for and pushed too.
    And which pull request I am going to link here in the next comment after I've created it.



---
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 [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] commons-compress issue #33: add-some-Unit-Tests Added some Unit Tests to inc...

ajs6f
In reply to this post by ajs6f
Github user TheRealHaui commented on the issue:

    https://github.com/apache/commons-compress/pull/33
 
    Link to the pull request: https://github.com/apache/commons-compress/pull/35


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] commons-compress issue #33: add-some-Unit-Tests Added some Unit Tests to inc...

ajs6f
In reply to this post by ajs6f
Github user coveralls commented on the issue:

    https://github.com/apache/commons-compress/pull/33
 
   
    [![Coverage Status](https://:/builds/12010056/badge)](https://:/builds/12010056)
   
    Coverage increased (+0.4%) to 84.713% when pulling **e0500a6b5f5f3b29c2b9417d20f5dc5966fa47f4 on TheRealHaui:add-some-Unit-Tests** into **0d8ab18c86f4edb38a73de1512483bb5b876bb1f 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 [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] commons-compress pull request #33: add-some-Unit-Tests Added some Unit Tests...

ajs6f
In reply to this post by ajs6f
Github user asfgit closed the pull request at:

    https://github.com/apache/commons-compress/pull/33


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] commons-compress issue #33: add-some-Unit-Tests Added some Unit Tests to inc...

ajs6f
In reply to this post by ajs6f
Github user bodewig commented on the issue:

    https://github.com/apache/commons-compress/pull/33
 
    Thanks 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 [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] commons-compress pull request #33: add-some-Unit-Tests Added some Unit Tests...

ajs6f
In reply to this post by ajs6f
Github user sesuncedu commented on a diff in the pull request:

    https://github.com/apache/commons-compress/pull/33#discussion_r122569687
 
    --- Diff: src/test/java/org/apache/commons/compress/compressors/xz/XZCompressorOutputStreamTest.java ---
    @@ -0,0 +1,51 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package org.apache.commons.compress.compressors.xz;
    +
    +import org.junit.Test;
    +
    +import java.io.ByteArrayOutputStream;
    +import java.io.IOException;
    +
    +import static org.junit.Assert.assertEquals;
    +
    +
    +/**
    + * Unit tests for class {@link XZCompressorOutputStream}.
    + *
    + * @date 16.06.2017
    + * @see XZCompressorOutputStream
    + **/
    +public class XZCompressorOutputStreamTest {
    +
    +
    +    @Test
    +    public void testWrite() throws IOException {
    +
    +        ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream(4590);
    +        XZCompressorOutputStream xZCompressorOutputStream = new XZCompressorOutputStream(byteArrayOutputStream);
    +        xZCompressorOutputStream.write(4590);
    +
    +        assertEquals(24, byteArrayOutputStream.size());
    +        assertEquals("\uFFFD7zXZ\u0000\u0000\u0004\uFFFD\u05B4F\u0002\u0000!\u0001\u0016\u0000\u0000\u0000t/\uFFFD", byteArrayOutputStream.toString());
    --- End diff --
   
    It's usually not a good idea to convert an arbitrary byte array to a string, as on can't rely on  a specific default  charset, and you may end up with characters mangled /mapped.
   
    I thinkThat hamcrest has array matchers; if not,  assertTrue(Arrays.equals(exp,act)) should be OK.
     I can't remember if there's an assertEquals for byte[]


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] commons-compress pull request #33: add-some-Unit-Tests Added some Unit Tests...

ajs6f
In reply to this post by ajs6f
Github user bodewig commented on a diff in the pull request:

    https://github.com/apache/commons-compress/pull/33#discussion_r122573708
 
    --- Diff: src/test/java/org/apache/commons/compress/compressors/xz/XZCompressorOutputStreamTest.java ---
    @@ -0,0 +1,51 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package org.apache.commons.compress.compressors.xz;
    +
    +import org.junit.Test;
    +
    +import java.io.ByteArrayOutputStream;
    +import java.io.IOException;
    +
    +import static org.junit.Assert.assertEquals;
    +
    +
    +/**
    + * Unit tests for class {@link XZCompressorOutputStream}.
    + *
    + * @date 16.06.2017
    + * @see XZCompressorOutputStream
    + **/
    +public class XZCompressorOutputStreamTest {
    +
    +
    +    @Test
    +    public void testWrite() throws IOException {
    +
    +        ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream(4590);
    +        XZCompressorOutputStream xZCompressorOutputStream = new XZCompressorOutputStream(byteArrayOutputStream);
    +        xZCompressorOutputStream.write(4590);
    +
    +        assertEquals(24, byteArrayOutputStream.size());
    +        assertEquals("\uFFFD7zXZ\u0000\u0000\u0004\uFFFD\u05B4F\u0002\u0000!\u0001\u0016\u0000\u0000\u0000t/\uFFFD", byteArrayOutputStream.toString());
    --- End diff --
   
    Thank you for catching this. I've reformulated the test with commit b53ead4 - I think we are not that much interested in the exact bytes created but rather that using `write` creates a valid XZ output.
   
    JUnit's `Assert` class contains `assertArrayEquals`.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]