jmeter-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject Re: About TestHTTPSamplersAgainstHttpMirrorServer
Date Sun, 15 Feb 2015 17:03:21 GMT
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.

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

Mime
View raw message