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 17:41:39 GMT
On Sun, Feb 15, 2015 at 6:03 PM, sebb <sebbaz@gmail.com> wrote:

> On 15 February 2015 at 16:53, Philippe Mouawad
> <philippe.mouawad@gmail.com> wrote:
> > 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.
> >
>
> Fine, but that is a separate issue.
>
> >
> >>
> >> >> 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 ?
>
> Yes.
>
> >
> >> 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 .
>
> I never said I was totally against the change.
>
> I said that I was against it _as it stands_, because it makes
> assumptions that are not tested.
>
> Changing the test as originally proposed makes the existing test worse
> UNLESS the assumptions underlying the change are also tested.
>
I don't believe so, but ok.
Regarding this test unless we expose impl this is not doable cleanly.

>
> >
> >
> >> 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.
>
> So that assumption needs to be tested.
>
> >
> >> 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.
>



-- 
Cordialement.
Philippe Mouawad.

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