harmony-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Andrew Zhang (JIRA)" <j...@apache.org>
Subject [jira] Commented: (HARMONY-691) [classlib][nio] Remove some unstable tests from o.a.h.tests.java.nio.channels.SocketChannelTest.
Date Thu, 29 Jun 2006 01:45:30 GMT
    [ http://issues.apache.org/jira/browse/HARMONY-691?page=comments#action_12418364 ] 

Andrew Zhang commented on HARMONY-691:
--------------------------------------

Hi George,

It depends on network, platform and machine. It sounds strange, but if you take a look at
these tests code, you will know the reason. In fact, if I remember correctly, one of these
tests has made Harmony build system failed. 

Let's take one test for example. (The others are similiar. ) Please see my comments inline.

  public void testReadByteBuffer_NonBlocking_ReadWriteRealTooLargeData()
            throws Exception {
        byte[] serverWBuf = new byte[CAPACITY_64KB];
        byte[] serverRBuf = new byte[CAPACITY_64KB + 1];
        for (int i = 0; i < serverWBuf.length; i++) {
            serverWBuf[i] = (byte) i;
        }
        java.nio.ByteBuffer buf = java.nio.ByteBuffer
                .allocate(CAPACITY_64KB + 1);
        this.channel1.configureBlocking(false);
        this.channel1.connect(localAddr1);
        Socket serverSocket = this.server1.accept();
        assertFalse(this.channel1.isConnected());
        if (tryFinish()) {
            OutputStream out = serverSocket.getOutputStream();
            InputStream in = serverSocket.getInputStream();
            out.write(serverWBuf);
            int count = 0;
            int total = 0;
            while (total < CAPACITY_64KB) {
                count = this.channel1.read(buf);
// Andrew comment 1: count <= 0 is not the right  break condition for non blocking read
operation.
// The reason you passed this test maybe because of your network condition, or something below
trick (not right way).
                if (count <= 0){
                    break;
                }
                total = total + count;
            }
// Andrew comment 2: assertEquals should be placed here instead of system.err.println(). 
// The  test passes even if the result is wrong(only with some system.out). It doesn't make
any sense.
// system.err.println is not recommended in test case. 
// I could only image it's useful only if the test can not be executed because of some environment
problems.
// But in this case, the result is wrong!
            if (CAPACITY_64KB == total) {
                buf.put((byte) 1);
                buf.flip();
                assertEquals(66051, buf.asIntBuffer().get());
            } else {
                System.err
                        .println("Read fail in capacity64KB, testReadByteBuffer_NonBlocking_ReadWriteRealTooLargeData
not finish.");
            }
            int writeCount = 0;
            writeCount = this.channel1.write(buf);
            assertTrue(CAPACITY_64KB + 1 >= writeCount);
            count = in.read(serverRBuf);
            while (total < CAPACITY_64KB + 1) {
                count = this.channel1.read(buf);
// The same as comment 1 
                if (count <= 0){
                    break;
                }
                total = total + count;
            }
// The same as comment 2. If the test acts wrong at comment 2 (system.err.println above),
but not fails above, 
// then following assertEquals would fail. 
            if (total > 0) {
                assertEquals(total, CAPACITY_64KB);
                for (int i = 0; i < count; i++) {
                    assertEquals((byte) i, serverRBuf[i]);
                }
            } else {
                System.err
                        .println("Read fail in capacity64KB+1, testReadByteBuffer_NonBlocking_ReadWriteRealTooLargeData
not finish.");
            }
        } else {
// The same as comment 2
            System.err
                    .println("connect fail, testReadByteBuffer_NonBlocking_ReadWriteRealTooLargeData
not finish.");
        }
// Andrew comment 3: Following part is unnecessary. This test is testing about read/write
operation ( in fact, each test  should keep small
// and fast, and is recommended to be focused on one function. This test tests read and write).

// It should be placed in test_close() method.
// In current version SocketChannelTest, the close() method is tested many times.
        this.channel1.close();
        try {
            assertEquals(CAPACITY_NORMAL, this.channel1.read(buf));
            fail("Should throw ClosedChannelException");
        } catch (ClosedChannelException e) {
            // correct
        }
    }

Therefore, I think there are several serious problems in these tests, which may potentially
cause Harmony system failed. 

Do you agree with me? 

I suggest remove them from tests, and I'll rewrite function tests for those removed part.


It's not a small task, so  I plan to repeat following steps until all of tests in SocketChannelTest
are stable.  
1. remove bad tests.
2. add function tests for removed part.

I devide SocketChannelTest into three parts:
1. read / write
2. some other system.err/out/println tests
3. others 

For each part, I'll raised a pair of seperated JIRAs to solve. 
What's your opnion?  Many thanks!

Best regards,
Andrew

> [classlib][nio] Remove some unstable tests from o.a.h.tests.java.nio.channels.SocketChannelTest.
> ------------------------------------------------------------------------------------------------
>
>          Key: HARMONY-691
>          URL: http://issues.apache.org/jira/browse/HARMONY-691
>      Project: Harmony
>         Type: Test

>   Components: Classlib
>     Reporter: Andrew Zhang
>     Assignee: George Harley
>  Attachments: nio.diff
>
> Hello, 
> I found some unstable tests in o.a.h.tests.java.nio.channels.SocketChannelTest. 
> I'll upload a patch to remove these unstable tests, and plan to add new stable tests
later  in a seperated JIRA. 
> Thanks!
> Best regards
> Andrew

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira


Mime
View raw message