You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jmeter.apache.org by Philippe Mouawad <ph...@gmail.com> on 2015/02/14 18:53:24 UTC

About TestHTTPSamplersAgainstHttpMirrorServer

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

I also suggest we replace new HttpSampler() by :
return new HTTPSamplerProxy(HTTPSamplerFactory.HTTP_SAMPLER_JAVA);
-- 
Regards
Philippe

Re: About TestHTTPSamplersAgainstHttpMirrorServer

Posted by sebb <se...@gmail.com>.
On 15 February 2015 at 17:41, Philippe Mouawad
<ph...@gmail.com> wrote:
> On Sun, Feb 15, 2015 at 6:03 PM, sebb <se...@gmail.com> wrote:
>
>> On 15 February 2015 at 16:53, Philippe Mouawad
>> <ph...@gmail.com> wrote:
>> > On Sunday, February 15, 2015, sebb <se...@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.

Re: About TestHTTPSamplersAgainstHttpMirrorServer

Posted by Philippe Mouawad <ph...@gmail.com>.
On Sun, Feb 15, 2015 at 6:03 PM, sebb <se...@gmail.com> wrote:

> On 15 February 2015 at 16:53, Philippe Mouawad
> <ph...@gmail.com> wrote:
> > On Sunday, February 15, 2015, sebb <se...@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.

Re: About TestHTTPSamplersAgainstHttpMirrorServer

Posted by sebb <se...@gmail.com>.
On 15 February 2015 at 16:53, Philippe Mouawad
<ph...@gmail.com> wrote:
> On Sunday, February 15, 2015, sebb <se...@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.

Re: About TestHTTPSamplersAgainstHttpMirrorServer

Posted by Philippe Mouawad <ph...@gmail.com>.
On Sunday, February 15, 2015, sebb <se...@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.

Re: About TestHTTPSamplersAgainstHttpMirrorServer

Posted by sebb <se...@gmail.com>.
On 15 February 2015 at 16:21, Philippe Mouawad
<ph...@gmail.com> wrote:
> On Sunday, February 15, 2015, sebb <se...@gmail.com> wrote:
>
>> On 14 February 2015 at 17:53, Philippe Mouawad
>> <philippe.mouawad@gmail.com <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.

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.

>> 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.
It could always return the JAVA sampler and AFAICT the existing tests
would not catch that.

There need to be tests that ensure HTTPSamplerProxy is working as expected.
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?

Re: About TestHTTPSamplersAgainstHttpMirrorServer

Posted by Philippe Mouawad <ph...@gmail.com>.
On Sunday, February 15, 2015, sebb <se...@gmail.com> wrote:

> On 14 February 2015 at 17:53, Philippe Mouawad
> <philippe.mouawad@gmail.com <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 ?

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

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

>
> > I also suggest we replace new HttpSampler() by :
> > return new HTTPSamplerProxy(HTTPSamplerFactory.HTTP_SAMPLER_JAVA);
>
> Where is this to be replaced?



>
> > --
> > Regards
> > Philippe
>


-- 
Cordialement.
Philippe Mouawad.

Re: About TestHTTPSamplersAgainstHttpMirrorServer

Posted by sebb <se...@gmail.com>.
On 14 February 2015 at 17:53, Philippe Mouawad
<ph...@gmail.com> 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.

> - 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.
So I don't think it is a good replacement without further tests to
check HTTPSamplerProxy actually does what one expects/

> I also suggest we replace new HttpSampler() by :
> return new HTTPSamplerProxy(HTTPSamplerFactory.HTTP_SAMPLER_JAVA);

Where is this to be replaced?

> --
> Regards
> Philippe