You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by akkumar <gi...@git.apache.org> on 2018/11/27 07:09:33 UTC

[GitHub] zookeeper pull request #726: ZOOKEEPER-3205: o.a.jute test cases

GitHub user akkumar opened a pull request:

    https://github.com/apache/zookeeper/pull/726

    ZOOKEEPER-3205: o.a.jute test cases

    Adding some test cases for o.a.jute BinaryInputArchive .  Issue ZOOKEEPER-3205 contains the patch as well. 

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

    $ git pull https://github.com/akkumar/zookeeper jutetest

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

    https://github.com/apache/zookeeper/pull/726.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 #726
    
----
commit 771d1d545dd0ef9799cbc1cdf7fcdbc48d675369
Author: Karthik K <ka...@...>
Date:   2018-11-26T16:21:43Z

    ZOOKEEPER-3205: o.a.jute test cases

----


---

[GitHub] zookeeper issue #726: ZOOKEEPER-3205: o.a.jute test cases

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

    https://github.com/apache/zookeeper/pull/726
  
    @eolivelli - import static added. Let me know if this looks ok !


---

[GitHub] zookeeper pull request #726: ZOOKEEPER-3205: o.a.jute test cases

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

    https://github.com/apache/zookeeper/pull/726#discussion_r236804588
  
    --- Diff: zookeeper-jute/src/test/java/org/apache/jute/BinaryInputArchiveTest.java ---
    @@ -41,4 +42,136 @@ public void testReadStringCheckLength() {
                         e.getMessage().startsWith(BinaryInputArchive.UNREASONBLE_LENGTH));
             }
         }
    +
    +    public interface TestWriter {
    +        void write(OutputArchive oa) throws IOException;
    +    }
    +
    +    public interface TestReader {
    +        void read(InputArchive ia) throws IOException;
    +    }
    +
    +    void checkWriterAndReader(TestWriter writer, TestReader reader) {
    +        ByteArrayOutputStream baos = new ByteArrayOutputStream();
    +        BinaryOutputArchive oa = BinaryOutputArchive.getArchive(baos);
    +        try {
    +            writer.write(oa);
    +        } catch (IOException e) {
    +            Assert.fail("Should not throw IOException");
    +        }
    +        InputStream is = new ByteArrayInputStream(baos.toByteArray());
    +        BinaryInputArchive ia = BinaryInputArchive.getArchive(is);
    +        try {
    +            reader.read(ia);
    +        } catch (IOException e) {
    +            Assert.fail("Should not throw IOException while reading back");
    +        }
    +    }
    +
    +    @Test
    +    public void testWriteInt() {
    +        final int expected = 4;
    +        final String tag = "tag1";
    +        checkWriterAndReader(
    +                new TestWriter() {
    --- End diff --
    
    Can we use lambdas?


---

[GitHub] zookeeper issue #726: ZOOKEEPER-3205: o.a.jute test cases

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

    https://github.com/apache/zookeeper/pull/726
  
    Only a few nits, other LGTM.


---

[GitHub] zookeeper pull request #726: ZOOKEEPER-3205: o.a.jute test cases

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

    https://github.com/apache/zookeeper/pull/726#discussion_r236957794
  
    --- Diff: zookeeper-jute/src/test/java/org/apache/jute/BinaryInputArchiveTest.java ---
    @@ -41,4 +42,136 @@ public void testReadStringCheckLength() {
                         e.getMessage().startsWith(BinaryInputArchive.UNREASONBLE_LENGTH));
             }
         }
    +
    +    public interface TestWriter {
    +        void write(OutputArchive oa) throws IOException;
    +    }
    +
    +    public interface TestReader {
    +        void read(InputArchive ia) throws IOException;
    +    }
    +
    +    void checkWriterAndReader(TestWriter writer, TestReader reader) {
    +        ByteArrayOutputStream baos = new ByteArrayOutputStream();
    +        BinaryOutputArchive oa = BinaryOutputArchive.getArchive(baos);
    +        try {
    +            writer.write(oa);
    +        } catch (IOException e) {
    +            Assert.fail("Should not throw IOException");
    +        }
    +        InputStream is = new ByteArrayInputStream(baos.toByteArray());
    +        BinaryInputArchive ia = BinaryInputArchive.getArchive(is);
    +        try {
    +            reader.read(ia);
    +        } catch (IOException e) {
    +            Assert.fail("Should not throw IOException while reading back");
    +        }
    +    }
    +
    +    @Test
    +    public void testWriteInt() {
    +        final int expected = 4;
    +        final String tag = "tag1";
    +        checkWriterAndReader(
    +                new TestWriter() {
    --- End diff --
    
    fixed. let me know if this looks ok ! 


---

[GitHub] zookeeper issue #726: ZOOKEEPER-3205: o.a.jute test cases

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

    https://github.com/apache/zookeeper/pull/726
  
    The test cases seem to run with no failures. 
    
    It is not obvious what caused the jenkins failure ? Any suggestions where to look for ? 


---

[GitHub] zookeeper pull request #726: ZOOKEEPER-3205: o.a.jute test cases

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

    https://github.com/apache/zookeeper/pull/726#discussion_r236977045
  
    --- Diff: zookeeper-jute/src/test/java/org/apache/jute/XmlInputArchiveTest.java ---
    @@ -0,0 +1,120 @@
    +/**
    + * 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
    + * <p/>
    + * http://www.apache.org/licenses/LICENSE-2.0
    + * <p/>
    + * 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.jute;
    +
    +import org.junit.Assert;
    +import org.junit.Test;
    +import org.xml.sax.SAXException;
    +
    +import javax.xml.parsers.ParserConfigurationException;
    +import java.io.ByteArrayOutputStream;
    +import java.io.ByteArrayInputStream;
    +import java.io.InputStream;
    +import java.io.IOException;
    +
    +public class XmlInputArchiveTest {
    +
    +    void checkWriterAndReader(TestWriter writer, TestReader reader) {
    +        ByteArrayOutputStream baos = new ByteArrayOutputStream();
    +
    +        try {
    +            XmlOutputArchive oa = XmlOutputArchive.getArchive(baos);
    +            writer.write(oa);
    +        } catch (IOException e) {
    +            Assert.fail("Should not throw IOException");
    +        }
    +        InputStream is = new ByteArrayInputStream(baos.toByteArray());
    +        try {
    +            XmlInputArchive ia = XmlInputArchive.getArchive(is);
    +            reader.read(ia);
    +        } catch (ParserConfigurationException e) {
    +            Assert.fail("Should not throw ParserConfigurationException while reading back");
    +        } catch (SAXException e) {
    +            Assert.fail("Should not throw SAXException while reading back");
    +        }  catch (IOException e) {
    +            Assert.fail("Should not throw IOException while reading back");
    +        }
    +    }
    +
    +    @Test
    +    public void testWriteInt() {
    +        final int expected = 4;
    +        final String tag = "tag1";
    +        checkWriterAndReader(
    +                (oa) -> oa.writeInt(expected, tag),
    +                (ia) -> {
    +                    int actual = ia.readInt(tag);
    +                    Assert.assertEquals(expected, actual);
    +                }
    +        );
    +    }
    +
    +    @Test
    +    public void testWriteBool() {
    +        final boolean expected = false;
    +        final String tag = "tag1";
    +        checkWriterAndReader(
    +                (oa) -> oa.writeBool(expected, tag),
    +                (ia) -> {
    +                    boolean actual = ia.readBool(tag);
    +                    Assert.assertEquals(expected, actual);
    --- End diff --
    
    Sorry, last nit: any reason for not using static imports?


---

[GitHub] zookeeper issue #726: ZOOKEEPER-3205: o.a.jute test cases

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

    https://github.com/apache/zookeeper/pull/726
  
    Ok - in the logs I see this.  Any suggestion what needs to be done to get this sorted out ? 
    
    ```
    
         [exec] 
    
         [exec] 
    
         [exec] Error: No value specified for option "issue"
    
         [exec] Session logged out. Session was JSESSIONID=....
    
         [exec] 
    
         [exec] 
    
         [exec] ======================================================================
    
         [exec] ======================================================================
    
         [exec]     Finished build.
    
         [exec] ======================================================================
    
         [exec] ======================================================================
    
         [exec] 
    
         [exec] 
    
         [exec] mv: '/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/patchprocess' and '/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/patchprocess' are the same file
    ```
    



---