harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Paulex Yang <paulex.y...@gmail.com>
Subject Re: [classlib] please review my regression test and patch for HttpURLConnection
Date Wed, 24 May 2006 07:20:26 GMT
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


Mime
View raw message