You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@harmony.apache.org by "Andrew Zhang (JIRA)" <ji...@apache.org> on 2006/06/28 12:12:29 UTC
[jira] Created: (HARMONY-691) [classlib][nio] Remove some unstable
tests from o.a.h.tests.java.nio.channels.SocketChannelTest.
[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
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
[jira] Commented: (HARMONY-691) [classlib][nio] Remove some
unstable tests from o.a.h.tests.java.nio.channels.SocketChannelTest.
Posted by "George Harley (JIRA)" <ji...@apache.org>.
[ http://issues.apache.org/jira/browse/HARMONY-691?page=comments#action_12418436 ]
George Harley commented on HARMONY-691:
---------------------------------------
Hi Andrew,
You make very good points on the quality of these tests. If you would allow me to try and summarise, it seems that you are highlighting two basic problems with them:
1) using stderr as a means of reporting potential failures while permitting the test to pass is poor.
2) the tests on non-blocking channels appear to be treating them in the same way as blocking channels. Again this is poor.
I agree that the tests under discussion should be re-written and will remove them for now pending the submission of replacement tests.
Best regards,
George
> [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
[jira] Commented: (HARMONY-691) [classlib][nio] Remove some
unstable tests from o.a.h.tests.java.nio.channels.SocketChannelTest.
Posted by "Andrew Zhang (JIRA)" <ji...@apache.org>.
[ http://issues.apache.org/jira/browse/HARMONY-691?page=comments#action_12418580 ]
Andrew Zhang commented on HARMONY-691:
--------------------------------------
Hi George,
The fix looks fine, Thank you very much!
I've raised a new JIRA Harmony-715 for submitting new tests.
Would you please have a check?
Thanks a lot!
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
[jira] Commented: (HARMONY-691) [classlib][nio] Remove some
unstable tests from o.a.h.tests.java.nio.channels.SocketChannelTest.
Posted by "Andrew Zhang (JIRA)" <ji...@apache.org>.
[ 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
[jira] Resolved: (HARMONY-691) [classlib][nio] Remove some unstable
tests from o.a.h.tests.java.nio.channels.SocketChannelTest.
Posted by "George Harley (JIRA)" <ji...@apache.org>.
[ http://issues.apache.org/jira/browse/HARMONY-691?page=all ]
George Harley resolved HARMONY-691:
-----------------------------------
Resolution: Fixed
Hi Andrew,
Patch committed in revision 418011. Please could you check that it has been applied as expected. I have also included SockChannelTest back into the nio tests to see how the remaining tests fare.
Looking forward to your new tests.
Best regards,
George
> [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
[jira] Updated: (HARMONY-691) [classlib][nio] Remove some unstable
tests from o.a.h.tests.java.nio.channels.SocketChannelTest.
Posted by "Andrew Zhang (JIRA)" <ji...@apache.org>.
[ http://issues.apache.org/jira/browse/HARMONY-691?page=all ]
Andrew Zhang updated HARMONY-691:
---------------------------------
Attachment: nio.diff
Hello,
Would you please try my patch?
Thanks a lot!
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
> 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
[jira] Commented: (HARMONY-691) [classlib][nio] Remove some
unstable tests from o.a.h.tests.java.nio.channels.SocketChannelTest.
Posted by "George Harley (JIRA)" <ji...@apache.org>.
[ http://issues.apache.org/jira/browse/HARMONY-691?page=comments#action_12418234 ]
George Harley commented on HARMONY-691:
---------------------------------------
Hi Andrew,
Please could you explain a bit more about the instability you are seeing around the tests that you propose removing ? Is the instability more or less likely depending on the runtime platform ? I've run them several hundred times on Windows XP (courtesy of the JUnit repeat decorator) and cannot get a failure.
Best regards,
George
> [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
[jira] Commented: (HARMONY-691) [classlib][nio] Remove some
unstable tests from o.a.h.tests.java.nio.channels.SocketChannelTest.
Posted by "Andrew Zhang (JIRA)" <ji...@apache.org>.
[ http://issues.apache.org/jira/browse/HARMONY-691?page=comments#action_12442160 ]
Andrew Zhang commented on HARMONY-691:
--------------------------------------
[[ Old comment, sent by email on Thu, 29 Jun 2006 09:46:25 +0800 ]]
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
--
Andrew Zhang
China Software Development Lab, IBM
> [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
> Issue Type: Test
> Components: Classlib
> Reporter: Andrew Zhang
> Assigned To: 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
[jira] Assigned: (HARMONY-691) [classlib][nio] Remove some unstable
tests from o.a.h.tests.java.nio.channels.SocketChannelTest.
Posted by "George Harley (JIRA)" <ji...@apache.org>.
[ http://issues.apache.org/jira/browse/HARMONY-691?page=all ]
George Harley reassigned HARMONY-691:
-------------------------------------
Assign To: George Harley
> [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
[jira] Closed: (HARMONY-691) [classlib][nio] Remove some unstable
tests from o.a.h.tests.java.nio.channels.SocketChannelTest.
Posted by "George Harley (JIRA)" <ji...@apache.org>.
[ http://issues.apache.org/jira/browse/HARMONY-691?page=all ]
George Harley closed HARMONY-691:
---------------------------------
Verified by 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