You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by Petr Shypila <ik...@gmail.com> on 2015/06/29 10:40:08 UTC
Writing unit tests for engine module
Hello Bertrand,
Recently I started to write unit tests for SlingMainServlet. And I have a
question there. It looks like it requires pretty much and deep mocking.
So my question is this okay just to check that some target action inside a
test void method was called, or I need to go deeper. I understand that it
depends on each particular case. But could you please take a look on my
first test for SlingMainServlet[1]. The reason why I ask you this is from
my point of view, deep mocking could take more time and if there is no such
need, I can skip it.
[1]
https://github.com/PetrShypila/sling-builder/blob/d4d04af973876aced8dd042ee9b3880ad4e00449/bundles/engine/src/test/java/org/apache/sling/engine/impl/SlingMainServletTest.java
Thank you,
Petr
Re: Writing unit tests for engine module
Posted by Bertrand Delacretaz <bd...@apache.org>.
Hi Petr,
On Wed, Jul 1, 2015 at 10:19 PM, Petr Shypila <ik...@gmail.com> wrote:
> ... I also wrote it in another way which
> becomes easier to understand[2]. Here I just mock objects on a first level
> and then check that they were called with correct parameters and I also
> check that these parameters have correct values...
This is much better and fairly readable, I agree.
The code at [2] however contains an lot of duplication, you should
refactor the common things to make it more readable and manageable.
I'm also not convinced about assertions such as
assertNotEquals(405, res.getStatus());
assertNotEquals("GET, HEAD, POST, PUT, DELETE, OPTIONS",
res.getHeader("Allow"));
It's not obvious at all what those tests, and they leave a whole range
of possible invalid values IMO - in general, tests should verify a
single or a small range of expected values. Here I suspect that your
tests make too many assumptions about the internal code, which might
make the tests brittle.
So in summary, this is going in the right direction but needs work!
-Bertrand
> [2]
> https://github.com/PetrShypila/sling-builder/blob/engine-module-simple/bundles/engine/src/test/java/org/apache/sling/engine/impl/SlingMainServletTest.java
>
> -Petr
>
> 2015-06-29 14:34 GMT+02:00 Petr Shypila <ik...@gmail.com>:
>
>> Yes, thank you. I'll do it in this way.
>>
>> Best Regards,
>> Petr Shypila
>>
>> > On 29 Jun 2015, at 14:23, Bertrand Delacretaz <bd...@apache.org>
>> wrote:
>> >
>> > Hi Petr,
>> >
>> >> On Mon, Jun 29, 2015 at 10:40 AM, Petr Shypila <ik...@gmail.com>
>> wrote:
>> >> ...could you please take a look on my
>> >> first test for SlingMainServlet[1]...
>> >
>> > I think it's a good start, but I don't think it's sufficient to just
>> > verify that the SlingRequestProcessor is called - you should verify
>> > that it is called with the appropriate values, and that its actions
>> > are taken into account. Best might be to mock the
>> > SlingRequestProcessor (or maybe it's easier to instantiate a
>> > SlingRequestProcessorImpl with a few mocks) and test both classes in
>> > combination, which is how they are used in practice.
>> >
>> > This diverges a bit from pure unit tests which should arguably test a
>> > single class, but we don't really care here IMO. Testing in a more
>> > realistic situation (so SlingMainServlet combined with
>> > SlingRequestProcessorImpl) will probably make your tests look more
>> > natural, which is good IMO.
>> >
>> > Does this work for you?
>> > -Bertrand
>> >
>> >
>> >> [1]
>> >>
>> https://github.com/PetrShypila/sling-builder/blob/d4d04af973876aced8dd042ee9b3880ad4e00449/bundles/engine/src/test/java/org/apache/sling/engine/impl/SlingMainServletTest.java
>>
Re: Writing unit tests for engine module
Posted by Petr Shypila <ik...@gmail.com>.
Hi Bertrand,
Some progress there. I tried to create real call
of SlingRequestProcessor#doProcessRequest(...), but it becomes pretty hard
to understand and even at this time I didn't create enough mocks to
launch doProcessRequest
method without exception[1]. So I also wrote it in another way which
becomes easier to understand[2]. Here I just mock objects on a first level
and then check that they were called with correct parameters and I also
check that these parameters have correct values. What do you think about
that?
[1]
https://github.com/PetrShypila/sling-builder/blob/engine-module/bundles/engine/src/test/java/org/apache/sling/engine/impl/SlingMainServletTest.java
[2]
https://github.com/PetrShypila/sling-builder/blob/engine-module-simple/bundles/engine/src/test/java/org/apache/sling/engine/impl/SlingMainServletTest.java
-Petr
2015-06-29 14:34 GMT+02:00 Petr Shypila <ik...@gmail.com>:
> Yes, thank you. I'll do it in this way.
>
> Best Regards,
> Petr Shypila
>
> > On 29 Jun 2015, at 14:23, Bertrand Delacretaz <bd...@apache.org>
> wrote:
> >
> > Hi Petr,
> >
> >> On Mon, Jun 29, 2015 at 10:40 AM, Petr Shypila <ik...@gmail.com>
> wrote:
> >> ...could you please take a look on my
> >> first test for SlingMainServlet[1]...
> >
> > I think it's a good start, but I don't think it's sufficient to just
> > verify that the SlingRequestProcessor is called - you should verify
> > that it is called with the appropriate values, and that its actions
> > are taken into account. Best might be to mock the
> > SlingRequestProcessor (or maybe it's easier to instantiate a
> > SlingRequestProcessorImpl with a few mocks) and test both classes in
> > combination, which is how they are used in practice.
> >
> > This diverges a bit from pure unit tests which should arguably test a
> > single class, but we don't really care here IMO. Testing in a more
> > realistic situation (so SlingMainServlet combined with
> > SlingRequestProcessorImpl) will probably make your tests look more
> > natural, which is good IMO.
> >
> > Does this work for you?
> > -Bertrand
> >
> >
> >> [1]
> >>
> https://github.com/PetrShypila/sling-builder/blob/d4d04af973876aced8dd042ee9b3880ad4e00449/bundles/engine/src/test/java/org/apache/sling/engine/impl/SlingMainServletTest.java
>
Re: Writing unit tests for engine module
Posted by Petr Shypila <ik...@gmail.com>.
Yes, thank you. I'll do it in this way.
Best Regards,
Petr Shypila
> On 29 Jun 2015, at 14:23, Bertrand Delacretaz <bd...@apache.org> wrote:
>
> Hi Petr,
>
>> On Mon, Jun 29, 2015 at 10:40 AM, Petr Shypila <ik...@gmail.com> wrote:
>> ...could you please take a look on my
>> first test for SlingMainServlet[1]...
>
> I think it's a good start, but I don't think it's sufficient to just
> verify that the SlingRequestProcessor is called - you should verify
> that it is called with the appropriate values, and that its actions
> are taken into account. Best might be to mock the
> SlingRequestProcessor (or maybe it's easier to instantiate a
> SlingRequestProcessorImpl with a few mocks) and test both classes in
> combination, which is how they are used in practice.
>
> This diverges a bit from pure unit tests which should arguably test a
> single class, but we don't really care here IMO. Testing in a more
> realistic situation (so SlingMainServlet combined with
> SlingRequestProcessorImpl) will probably make your tests look more
> natural, which is good IMO.
>
> Does this work for you?
> -Bertrand
>
>
>> [1]
>> https://github.com/PetrShypila/sling-builder/blob/d4d04af973876aced8dd042ee9b3880ad4e00449/bundles/engine/src/test/java/org/apache/sling/engine/impl/SlingMainServletTest.java
Re: Writing unit tests for engine module
Posted by Bertrand Delacretaz <bd...@apache.org>.
Hi Petr,
On Mon, Jun 29, 2015 at 10:40 AM, Petr Shypila <ik...@gmail.com> wrote:
> ...could you please take a look on my
> first test for SlingMainServlet[1]...
I think it's a good start, but I don't think it's sufficient to just
verify that the SlingRequestProcessor is called - you should verify
that it is called with the appropriate values, and that its actions
are taken into account. Best might be to mock the
SlingRequestProcessor (or maybe it's easier to instantiate a
SlingRequestProcessorImpl with a few mocks) and test both classes in
combination, which is how they are used in practice.
This diverges a bit from pure unit tests which should arguably test a
single class, but we don't really care here IMO. Testing in a more
realistic situation (so SlingMainServlet combined with
SlingRequestProcessorImpl) will probably make your tests look more
natural, which is good IMO.
Does this work for you?
-Bertrand
> [1]
> https://github.com/PetrShypila/sling-builder/blob/d4d04af973876aced8dd042ee9b3880ad4e00449/bundles/engine/src/test/java/org/apache/sling/engine/impl/SlingMainServletTest.java