Hello,
I fixed a bug in geoserver app-schema (https://github.com/geotools/geotools/pull/3367 https://osgeo-org.atlassian.net/browse/GEOT-6801). The change I made was very small (one character actually) and in one specific class. To me it seemed appropriate to write a unit test. Since it is JDBC an integration test would only be run online with db, but still require a lot of amount of work to create the right table, mapping etc... Mocking seems useful to me.
Anyway, because one of the classes I need is final, I had to use PowerMock to write the test. The Reviewer of the patch ask to check with the list if it is okay to add the PowerMock library to the test scope. I know traditionally we have more integration tests than unit tests, but I think it wouldn't be bad to add more unit tests in general.
Kind Regards
Niels
On 08-03-2021 13:01, Niels Charlier via Geoserver-devel wrote:
Hello,
I fixed a bug in geoserver app-schema (https://github.com/geotools/geotools/pull/3367 https://osgeo-org.atlassian.net/browse/GEOT-6801). The change I made was very small (one character actually) and in one specific class. To me it seemed appropriate to write a unit test. Since it is JDBC an integration test would only be run online with db, but still require a lot of amount of work to create the right table, mapping etc... Mocking seems useful to me.
searching the code [1] shows we have many, many mocking frameworks already EasyMock, PowerMock, Mockito, WireMock, ... some of which are only used in one unsupported module.
It seems powermock is only used in unsupported modules, so my preference would be to not add this to the core but use one of the other mocking frameworks already used in the core if possible.
[1] https://github.com/geotools/geotools/search?q=mock
Mark
PowerMock is the only mocking library that can bypass a class being final. It actually serves a different purpose to these other frameworks because you must still use the API of one of the other ones (easymock, mockito,..) of your choice, but you get additional 'power'.
The alternative would be to avoid using 'final' on a class so that we can still mock those classes.
Kind Regards
Niels
On 09/03/2021 11:40, Mark Prins wrote:
On 08-03-2021 13:01, Niels Charlier via Geoserver-devel wrote:
Hello,
I fixed a bug in geoserver app-schema (https://github.com/geotools/geotools/pull/3367 https://osgeo-org.atlassian.net/browse/GEOT-6801). The change I made was very small (one character actually) and in one specific class. To me it seemed appropriate to write a unit test. Since it is JDBC an integration test would only be run online with db, but still require a lot of amount of work to create the right table, mapping etc... Mocking seems useful to me.
searching the code [1] shows we have many, many mocking frameworks already EasyMock, PowerMock, Mockito, WireMock, ... some of which are only used in one unsupported module.
It seems powermock is only used in unsupported modules, so my preference would be to not add this to the core but use one of the other mocking frameworks already used in the core if possible.
[1] https://github.com/geotools/geotools/search?q=mock
Mark
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
Hi all,
chiming in… indeed Nuno’s comment was not against unit testing with mocks (that’s good per se),
but about the proliferation of mock testing libraries:
https://github.com/geotools/geotools/pull/3367#issuecomment-791476559
I also agree there are too many.
By the way, just a note, wiremock solves a different problem than the other mocking libraries,
it provides an actual HTTP server that can be used to test systems which do not allow injection
of a local mock.
Cheers
Andrea
···
Regards, Andrea Aime
== GeoServer Professional Services from the experts! Visit http://goo.gl/it488V for more information. == Ing. Andrea Aime @geowolf Technical Lead GeoSolutions S.A.S. Via di Montramito 3/A 55054 Massarosa (LU) phone: +39 0584 962313 fax: +39 0584 1660272 mob: +39 339 8844549 http://www.geo-solutions.it http://twitter.com/geosolutions_it ------------------------------------------------------- Con riferimento alla normativa sul trattamento dei dati personali (Reg. UE 2016/679 - Regolamento generale sulla protezione dei dati “GDPR”), si precisa che ogni circostanza inerente alla presente email (il suo contenuto, gli eventuali allegati, etc.) è un dato la cui conoscenza è riservata al/i solo/i destinatario/i indicati dallo scrivente. Se il messaggio Le è giunto per errore, è tenuta/o a cancellarlo, ogni altra operazione è illecita. Le sarei comunque grato se potesse darmene notizia. This email is intended only for the person or entity to which it is addressed and may contain information that is privileged, confidential or otherwise protected from disclosure. We remind that - as provided by European Regulation 2016/679 “GDPR” - copying, dissemination or use of this e-mail or the information herein by anyone other than the intended recipient is prohibited. If you have received this email by mistake, please notify us immediately by telephone or e-mail.
Another consideration… what is the “popularity” of the mocking libraries in the code?
I made a very quick check (yes, debatable, I know):
~/devel/git-gt (main) $ git grep mockito | wc -l
114
~/devel/git-gt (main) $ git grep easymock | wc -l
67
~/devel/git-gt (main) $ git grep powermock | wc -l
23
Then I looked at Mockito, and it seems we’re depending on a fairly recent version.
And then stumbled into this:
https://www.baeldung.com/mockito-final
Hmm… so maybe Mockito too can handle final classes?
Cheers
Andrea
···
Regards, Andrea Aime
== GeoServer Professional Services from the experts! Visit http://goo.gl/it488V for more information. == Ing. Andrea Aime @geowolf Technical Lead GeoSolutions S.A.S. Via di Montramito 3/A 55054 Massarosa (LU) phone: +39 0584 962313 fax: +39 0584 1660272 mob: +39 339 8844549 http://www.geo-solutions.it http://twitter.com/geosolutions_it ------------------------------------------------------- Con riferimento alla normativa sul trattamento dei dati personali (Reg. UE 2016/679 - Regolamento generale sulla protezione dei dati “GDPR”), si precisa che ogni circostanza inerente alla presente email (il suo contenuto, gli eventuali allegati, etc.) è un dato la cui conoscenza è riservata al/i solo/i destinatario/i indicati dallo scrivente. Se il messaggio Le è giunto per errore, è tenuta/o a cancellarlo, ogni altra operazione è illecita. Le sarei comunque grato se potesse darmene notizia. This email is intended only for the person or entity to which it is addressed and may contain information that is privileged, confidential or otherwise protected from disclosure. We remind that - as provided by European Regulation 2016/679 “GDPR” - copying, dissemination or use of this e-mail or the information herein by anyone other than the intended recipient is prohibited. If you have received this email by mistake, please notify us immediately by telephone or e-mail.
Yes, I managed to get rid of powermock and use mockito this way. Thanks.
As a side note though, you can’t treat powermock as an alternative for those other two. Powermock is always used in combination with (and depending on) one of those two (you must use either powermock-easymock or powermockito), it adds support for final classes, but also for example private and static methods. . Everyone who used powermock also used mockito or easymock. Mockito and easymock are however always entirely replaceable for each other.
Considering the popularity, if you’d want to clean things up, it would make sense to get rid in the first place of easymock and powermock-easymock, and only use mockito and powermockito. Then you’d have the same mock API everywhere and less dependencies. You wouldn’t so easily be able to get rid of powermock, because people would have used it for a reason.
Kind Regards
Niels
···
On 09/03/2021 12:09, Andrea Aime wrote:
Another consideration… what is the “popularity” of the mocking libraries in the code?
I made a very quick check (yes, debatable, I know):
~/devel/git-gt (main) $ git grep mockito | wc -l
114
~/devel/git-gt (main) $ git grep easymock | wc -l
67
~/devel/git-gt (main) $ git grep powermock | wc -l
23
Then I looked at Mockito, and it seems we’re depending on a fairly recent version.
And then stumbled into this:
https://www.baeldung.com/mockito-final
Hmm… so maybe Mockito too can handle final classes?
Cheers
Andrea
On Tue, Mar 9, 2021 at 11:49 AM Andrea Aime <andrea.aime@anonymised.com> wrote:
Hi all,
chiming in… indeed Nuno’s comment was not against unit testing with mocks (that’s good per se),
but about the proliferation of mock testing libraries:
https://github.com/geotools/geotools/pull/3367#issuecomment-791476559
I also agree there are too many.
By the way, just a note, wiremock solves a different problem than the other mocking libraries,
it provides an actual HTTP server that can be used to test systems which do not allow injection
of a local mock.
Cheers
Andrea
On Tue, Mar 9, 2021 at 11:42 AM Mark Prins <mc.prins@anonymised.com> wrote:
On 08-03-2021 13:01, Niels Charlier via Geoserver-devel wrote:
Hello,
I fixed a bug in geoserver app-schema
(https://github.com/geotools/geotools/pull/3367
https://osgeo-org.atlassian.net/browse/GEOT-6801). The change I made was
very small (one character actually) and in one specific class. To me it
seemed appropriate to write a unit test. Since it is JDBC an integration
test would only be run online with db, but still require a lot of amount
of work to create the right table, mapping etc… Mocking seems useful
to me.
searching the code [1] shows we have many, many mocking frameworks
already EasyMock, PowerMock, Mockito, WireMock, … some of which are
only used in one unsupported module.
It seems powermock is only used in unsupported modules, so my preference
would be to not add this to the core but use one of the other mocking
frameworks already used in the core if possible.
[1] https://github.com/geotools/geotools/search?q=mock
Mark
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
–
Regards, Andrea Aime
== GeoServer Professional Services from the experts! Visit http://goo.gl/it488V for more information. == Ing. Andrea Aime @geowolf Technical Lead GeoSolutions S.A.S. Via di Montramito 3/A 55054 Massarosa (LU) phone: +39 0584 962313 fax: +39 0584 1660272 mob: +39 339 8844549 http://www.geo-solutions.it http://twitter.com/geosolutions_it ------------------------------------------------------- Con riferimento alla normativa sul trattamento dei dati personali (Reg. UE 2016/679 - Regolamento generale sulla protezione dei dati “GDPR”), si precisa che ogni circostanza inerente alla presente email (il suo contenuto, gli eventuali allegati, etc.) è un dato la cui conoscenza è riservata al/i solo/i destinatario/i indicati dallo scrivente. Se il messaggio Le è giunto per errore, è tenuta/o a cancellarlo, ogni altra operazione è illecita. Le sarei comunque grato se potesse darmene notizia. This email is intended only for the person or entity to which it is addressed and may contain information that is privileged, confidential or otherwise protected from disclosure. We remind that - as provided by European Regulation 2016/679 “GDPR” - copying, dissemination or use of this e-mail or the information herein by anyone other than the intended recipient is prohibited. If you have received this email by mistake, please notify us immediately by telephone or e-mail.
–
Regards, Andrea Aime
== GeoServer Professional Services from the experts! Visit http://goo.gl/it488V for more information. == Ing. Andrea Aime @geowolf Technical Lead GeoSolutions S.A.S. Via di Montramito 3/A 55054 Massarosa (LU) phone: +39 0584 962313 fax: +39 0584 1660272 mob: +39 339 8844549 http://www.geo-solutions.it http://twitter.com/geosolutions_it ------------------------------------------------------- Con riferimento alla normativa sul trattamento dei dati personali (Reg. UE 2016/679 - Regolamento generale sulla protezione dei dati “GDPR”), si precisa che ogni circostanza inerente alla presente email (il suo contenuto, gli eventuali allegati, etc.) è un dato la cui conoscenza è riservata al/i solo/i destinatario/i indicati dallo scrivente. Se il messaggio Le è giunto per errore, è tenuta/o a cancellarlo, ogni altra operazione è illecita. Le sarei comunque grato se potesse darmene notizia. This email is intended only for the person or entity to which it is addressed and may contain information that is privileged, confidential or otherwise protected from disclosure. We remind that - as provided by European Regulation 2016/679 “GDPR” - copying, dissemination or use of this e-mail or the information herein by anyone other than the intended recipient is prohibited. If you have received this email by mistake, please notify us immediately by telephone or e-mail.
_______________________________________________
Geoserver-devel mailing list
[Geoserver-devel@lists.sourceforge.net](mailto:Geoserver-devel@lists.sourceforge.net)
[https://lists.sourceforge.net/lists/listinfo/geoserver-devel](https://lists.sourceforge.net/lists/listinfo/geoserver-devel)