jmeter-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Philippe Mouawad <philippe.moua...@gmail.com>
Subject Re: About TestHTTPSamplersAgainstHttpMirrorServer
Date Sun, 15 Feb 2015 16:53:05 GMT
On Sunday, February 15, 2015, sebb <sebbaz@gmail.com> wrote:

> On 15 February 2015 at 16:21, Philippe Mouawad
> <philippe.mouawad@gmail.com <javascript:;>> wrote:
> > On Sunday, February 15, 2015, sebb <sebbaz@gmail.com <javascript:;>>
> wrote:
> >
> >> On 14 February 2015 at 17:53, Philippe Mouawad
> >> <philippe.mouawad@gmail.com <javascript:;> <javascript:;>>
wrote:
> >> > Hello,
> >> > Looking at code of class, I see that in fact HTTPSamplerProxy which is
> >> used
> >> > by Http Sampler is not tested , instead it appears the class tests :
> >> > - HTTPSampler3 (which is not used anywhere anymore, I propose to
> delete
> >> it,
> >> > I will create a bugzilla for this )
> >>
> >> HTTPSampler3 is currently used by test code.
> >>
> >>  Why not just use
> HTTPSamplerProxy(HTTPSamplerFactory.IMPL_HTTP_CLIENT4);
> > instead ?
>
> You said it was not used. But it is.

I meant in non test code but ok :)


>
> Replacing it with a call to HTTPSamplerProxy is a separate issue.
>
> >> - HTTPSampler2 (used by SoapSampler)
> >> > - HTTPSampler
> >> >
> >> >
> >> > I suggest we replace code by :
> >> >     private HTTPSamplerBase createHttpSampler(int samplerType) {
> >> >         switch(samplerType) {
> >> >             case HTTP_SAMPLER:
> >> >                 return new
> >> > HTTPSamplerProxy(HTTPSamplerFactory.HTTP_SAMPLER_JAVA);
> >> >             case HTTP_SAMPLER2:
> >> >                 return new
> >> > HTTPSamplerProxy(HTTPSamplerFactory.IMPL_HTTP_CLIENT3_1);
> >> >             case HTTP_SAMPLER3:
> >> >                 return new
> >> > HTTPSamplerProxy(HTTPSamplerFactory.IMPL_HTTP_CLIENT4);
> >> >             default:
> >> >                 break;
> >> >         }
> >> >         throw new IllegalArgumentException("Unexpected type:
> >> "+samplerType);
> >> >     }
> >> >
> >> > If everybody is OK , I will create a bugzilla for this.
> >>
> >> That assumes HTTPSamplerProxy is working OK.
> >
> > I don't understand what you mean here.
> > Shouldn't this class be the one under kunit tests instead of ones that
> are
> > rarely or not used.
>
> Probably, but again that is a slightly different issue.


I don't share your view. My purpose here is to point that some junit code
is not doing tests on classes that are part of jmeter core and part of it
are doing it on deprecated code.



>
> >> So I don't think it is a good replacement without further tests to
> >> check HTTPSamplerProxy actually does what one expects/
> >
> >
> > I don't understand sebb.
>
> There are no tests to ensure that HTTPSamplerProxy actually generates
> the correct sampler.


You mean in terms of implementation returned ?


> It could always return the JAVA sampler and AFAICT the existing tests
> would not catch that.


Ok we could improve this, at least doing the change I propose would improve
things by applying tests on most used jmeter class. Overall as long as it
improves things why are you against that change .



> There need to be tests that ensure HTTPSamplerProxy is working as expected.

as its logic is rather simple it could easily be added. It's true that I
suppose it works.


> Otherwise tests that depend on it working correctly are not very useful.
>
> >>
> >> > I also suggest we replace new HttpSampler() by :
> >> > return new HTTPSamplerProxy(HTTPSamplerFactory.HTTP_SAMPLER_JAVA);
> >>
> >> Where is this to be replaced?
>


-- 
Cordialement.
Philippe Mouawad.

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