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 18:14:39 GMT
On 15 February 2015 at 17:41, Philippe Mouawad
<philippe.mouawad@gmail.com> wrote:
> 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.

It's not a question of belief - this can be proved.

Change the code as follows:

    private HTTPSamplerBase createHttpSampler(int samplerType) {
        switch(samplerType) {
            case HTTP_SAMPLER:
                return new HTTPSampler2(); // wrong sampler
...

This results in test failures, but the failure messages give no clue
as to the cause of the problem.
This is an example of what could happen if HTTPSamplerProxy were to
use an incorrect implementation.

> Regarding this test unless we expose impl this is not doable cleanly.

Then either we have to find a way to test the proxy, or we don't
change the existing test.

>>
>> >
>> >
>> >> 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
View raw message