Mailing-List: contact harmony-dev-help@incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: harmony-dev@incubator.apache.org
Received-SPF: neutral (asf.osuosl.org: 202.81.18.154 is neither permitted nor
 denied by domain of paulex.yang@gmail.com)
Message-ID: <4474093A.3010802@gmail.com>
Date: Wed, 24 May 2006 15:20:26 +0800
From: Paulex Yang <paulex.yang@gmail.com>
User-Agent: Thunderbird 1.5.0.2 (Windows/20060308)
MIME-Version: 1.0
To: harmony-dev@incubator.apache.org
Subject: Re: [classlib] please review my regression test and patch for
 HttpURLConnection
References: <906dd82e0605230154j757fc3c5r82a36fdd17cd3297@mail.gmail.com>
In-Reply-To: <906dd82e0605230154j757fc3c5r82a36fdd17cd3297@mail.gmail.com>
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset=ISO-8859-1; format=flowed

Mikhail,

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

If I understand correctly, Harmony-482 is about the internal contract 
between java.net.HttpURLConnection.setRequestMethod() and 
o.a.h.l.internal.net.www.protocol.http.HttpURLConnection.getOutputStream(), 
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 
URL("http://127.0.0.1:"
                + port).openConnection();
        c.setDoOutput(true);
        c.setRequestMethod(new String("GET"));
        c.getOutputStream();

        ...//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 over-designed. 

However, I even have no strong objection on it. What makes me more 
confused is the introduction of injected 
java.net.HttpURLConnectionAccessor, 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 
catch the fragility of the internal contract.


Mikhail Loenko wrote:
> 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 Lab
IBM


---------------------------------------------------------------------
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