harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Yang Paulex" <paulex.y...@gmail.com>
Subject Re: [classlib] please review my regression test and patch for HttpURLConnection
Date Thu, 25 May 2006 09:27:21 GMT

Sorry to response so late, seems both IBM's smtp server and gmail is not
stable so that I have to send this mail several times from today before
yesterday (please pardon me if the mailing list get several duplicate mails
after hundreds of hours! ).

Basically, I'm afraid I cannot agree with you on this patch.

1. I have no strong objection on the patch for HttpURLConnection, but...If I
understand correctly, Harmony-482 is about the internal contract between
java.net.HttpURLConnection.setRequestMethod() and
and the internal contract may be broken only if some of the codes refactored
(no chance for user application), say, modification for the
setRequestMethod() like this:

-               this.method = methodTokens[i];
+                this.method = method;

But this can be easily monitored by test codes below:

   public void testGetOutputStream() throws Exception {
       HttpURLConnection c = (HttpURLConnection) new
               + port).openConnection();
       c.setRequestMethod(new String("GET"));

       ...//other for POST/PUT

This test is enough to enforced the internal contract, it will shout if some
refactory breaks the internal contract. So IMHO, I think the patch for
HttpURLConnection is a little over-designed.

2. Even the patch is necessary, I think the introduction of injected
java.net.HttpURLConnectionAccessor is debatable. I suggest to avoid the
injected helper class as long as possible, because it introduces complexity
to understand/manage and is sometimes fragile because it may highly coupled
with implementation details. For this specific case, a mocked subclass of
o.a.h....HttpURLConnection  with overridden getRequestMethod can be used

Your comments?

2006/5/23, Mikhail Loenko <mloenko@gmail.com>:
> I've created regression test and patch for
> http://issues.apache.org/jira/browse/HARMONY-482
> I had to make some changes in the luni's build.xml,
> that are intended to be fixed once we all agree with
> proposed test suite layout.
> Please review the changes.
> Thanks,
> Mikhail
> ---------------------------------------------------------------------
> Terms of use : http://incubator.apache.org/harmony/mailing.html
> To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> For additional commands, e-mail: harmony-dev-help@incubator.apache.org

Paulex Yang
China Software Development Labotary

  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message