You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by bu...@apache.org on 2005/12/21 07:07:04 UTC
DO NOT REPLY [Bug 37985] New: - unit tests fail for commons-net-1.4.1 with NullPointerException
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=37985>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.
http://issues.apache.org/bugzilla/show_bug.cgi?id=37985
Summary: unit tests fail for commons-net-1.4.1 with
NullPointerException
Product: Commons
Version: unspecified
Platform: PC
URL: https://bugs.gentoo.org/show_bug.cgi?id=116226
OS/Version: Linux
Status: NEW
Severity: normal
Priority: P2
Component: Net
AssignedTo: commons-dev@jakarta.apache.org
ReportedBy: nichoj@gentoo.org
I'm working on updating the commons-net package for Gentoo to the most recent
version, 1.4.1, and ran into a problem running the unit tests:
[junit] Testsuite: org.apache.commons.net.time.TimeTCPClientTest
[junit] Tests run: 2, Failures: 0, Errors: 1, Time elapsed: 1.093 sec
[junit] Testcase: testInitial took 0.009 sec
[junit] Testcase: testCompareTimes took 1.074 sec
[junit] Caused an ERROR
[junit] null
[junit] java.lang.NullPointerException
[junit] at
org.apache.commons.net.SocketClient.disconnect(SocketClient.java:266)
[junit] at
org.apache.commons.net.time.TimeTCPClientTest.testCompareTimes(TimeTCPClientTest.java:133)
Upon further investigation into SocketClient, I found that disconnect() tried to
close a few objects, without checking that they weren't null. Adding a simple
check for null is enough to get the tests to pass.
--
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
DO NOT REPLY [Bug 37985] - unit tests fail for commons-net-1.4.1 with NullPointerException
Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=37985>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.
http://issues.apache.org/bugzilla/show_bug.cgi?id=37985
nichoj@gentoo.org changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|RESOLVED |REOPENED
Resolution|INVALID |
------- Additional Comments From nichoj@gentoo.org 2005-12-21 23:38 -------
(In reply to comment #2)
> disconnect should not be called on a SocketClient instance that has not
> been successfully connected. If there's a problem, it would be
> with the unit test, not SocketClient. In other words, the test code should be
> if(client.isConnected())
> client.disconnect();
> instead of
> client.disconnect();
> However, if you do that, then the unit test doesn't fail when a connection
> is not established. So you could write
> if(client.isConnected())
> client.disconnect();
> else
> throw SomeAppropriateException("Connection failed.");
>
> But since client.disconnect() already throws an exception, I have to assume
> the test writer opted for the shorter
> client.disconnect();
Fair enough. However, I think that some exception should be thrown (maybe
IllegalStateException?) that indicates that the object is in an invalid state,
instead of letting a NullPointerException occur.
>
> In short, the test is supposed to fail if it can't connect.
I can accept that the test would fail if it can't connect. However, it really
shouldn't be because of a NullPointerException.
(In reply to comment #3)
> I don't mean to speak for the rest of the Commons Net developers, but I think
> we'd rather you package it by bypassing the unit tests during packaging.
I concurr about not including the patch. We actually prefer to keep packages as
close to upstream as possible.
> Applying your patch may cause more bug reports from people who call disconnect()
> improperly. With the patch, they'll have no indication they did something wrong.
> We can discuss this further on commons-dev.
The current code doesn't give much better of an indication that something went
wrong, so perhaps throwing an exception, like I mentioned above, is more
appropriate?
--
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
DO NOT REPLY [Bug 37985] - unit tests fail for commons-net-1.4.1 with NullPointerException
Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=37985>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.
http://issues.apache.org/bugzilla/show_bug.cgi?id=37985
------- Additional Comments From dfs@apache.org 2005-12-21 23:24 -------
(In reply to comment #1)
> This patch solves the problem. I'm going to go ahead with packaging 1.4.1 using
> this patch, unless there's some overwhelming reason not to.
I don't mean to speak for the rest of the Commons Net developers, but I think
we'd rather you package it by bypassing the unit tests during packaging.
Applying your patch may cause more bug reports from people who call disconnect()
improperly. With the patch, they'll have no indication they did something wrong.
We can discuss this further on commons-dev.
--
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
DO NOT REPLY [Bug 37985] - unit tests fail for commons-net-1.4.1 with NullPointerException
Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=37985>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.
http://issues.apache.org/bugzilla/show_bug.cgi?id=37985
------- Additional Comments From nichoj@gentoo.org 2005-12-21 07:10 -------
Created an attachment (id=17250)
--> (http://issues.apache.org/bugzilla/attachment.cgi?id=17250&action=view)
commons-net-1.4.1-socketclient.patch
This patch solves the problem. I'm going to go ahead with packaging 1.4.1 using
this patch, unless there's some overwhelming reason not to.
--
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
DO NOT REPLY [Bug 37985] - [net] Unit tests fail for commons-net-1.4.1 with NullPointerException
Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=37985>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.
http://issues.apache.org/bugzilla/show_bug.cgi?id=37985
dfs@apache.org changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|REOPENED |RESOLVED
Resolution| |FIXED
------- Additional Comments From dfs@apache.org 2005-12-23 22:30 -------
I changed the test to check isConnected before calling disconnect. My
original statement about what the original test writer probably intended
was incorrect. connect() throws an exception when it fails. The lack
of any catch block indicates the test writer intended for the original
exception to pass through. However, in that context--by placing the
disconnect in a finally block--it is incorrect for the test to call
disconnect without first checking isConnected. Sorry for my original
misdiagnosis.
With regard to SocketClient, I have no objection to making disconnect throw
an IllegalStateException if that is where the consensus of the Commons Net
user and developer community has moved. However, that would be an
enhancement request and should be discussed on the commons-dev mailing
list first. The reason an IllegalStateException is not thrown is because
the library was designed with the philosophy of not checking every possible
API misuse and throwing an exception in each case. Doing so would have
littered the code with a lot of if then else throw sequences, making the
API throw even more exceptions than it already does. The consensus in the
past has been to not check every possible mistake in API use. However,
that consensus can be retested on commons-dev.
--
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
DO NOT REPLY [Bug 37985] - unit tests fail for commons-net-1.4.1 with NullPointerException
Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=37985>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.
http://issues.apache.org/bugzilla/show_bug.cgi?id=37985
dfs@apache.org changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution| |INVALID
------- Additional Comments From dfs@apache.org 2005-12-21 23:17 -------
disconnect should not be called on a SocketClient instance that has not
been successfully connected. If there's a problem, it would be
with the unit test, not SocketClient. In other words, the test code should be
if(client.isConnected())
client.disconnect();
instead of
client.disconnect();
However, if you do that, then the unit test doesn't fail when a connection
is not established. So you could write
if(client.isConnected())
client.disconnect();
else
throw SomeAppropriateException("Connection failed.");
But since client.disconnect() already throws an exception, I have to assume
the test writer opted for the shorter
client.disconnect();
In short, the test is supposed to fail if it can't connect.
--
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
DO NOT REPLY [Bug 37985] - [net] Unit tests fail for commons-net-1.4.1 with NullPointerException
Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=37985>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.
http://issues.apache.org/bugzilla/show_bug.cgi?id=37985
------- Additional Comments From scohen@apache.org 2005-12-24 02:38 -------
My only comment here is that you've patched the head which is not the same as
patching version 1.4.1. This is exactly the right thing to have done, I am not
criticizing at all but I just want to clarify this point since the summary
mentions 1.4.1 specifically.
Version 1.4.1 was handled as a BRANCH off 1.4.0, rather than the usual release
off the latest HEAD. I did it this way because we were getting frequent
complaints about our binary packages being incompatible with JDK 1.3, and I was
pressed for time and it was easier. 1.4.1 fixes the 1.3 JDK problem and only
that problem. It doesn't even include changes made to the HEAD since 1.4.0
before the release of 1.4.1.
I mainly bring this up because we're probably approaching the point where we
should think about making a proper 1.4.2 release that releases all the fixed
defects we've accumulated over the past 7 or 8 months.
--
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
DO NOT REPLY [Bug 37985] - [net] Unit tests fail for commons-net-1.4.1 with NullPointerException
Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=37985>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.
http://issues.apache.org/bugzilla/show_bug.cgi?id=37985
ebourg@apache.org changed:
What |Removed |Added
----------------------------------------------------------------------------
Summary|unit tests fail for commons-|[net] Unit tests fail for
|net-1.4.1 with |commons-net-1.4.1 with
|NullPointerException |NullPointerException
--
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org