Hi, in the last few days I've been working on switching our large mass of JUnit 3 tests to JUnit 4.
The original plan was simple: let IntelliJ do most of the job, with its automatic migration, and then fill in the blanks. Turned out IntelliJ is very good at migrating simple JUnit tests, but not tests that inherit from other base test classes... which is a pattern very widely used in GeoTools. Long story short, IntelliJ migrated some 250 tests, but I had to modify more or less twice as that by "hand"... where hand means, mostly regex replacements with extra actual "by hand" changes. The final PR modifies 870 files: https://github.com/geotools/geotools/pull/3306 It was an epic endeavour. Some highlights follow: - Classes had a lot of needless cruft, like suite methods manually collecting the test methods, or "main" with runners. Wiped them all out. - JUnit3 had an equals between doubles without a tolerance, switching to JUnit4 all these calls required the tolerance param to be added (the old method is there, but deprecated) - The test compile in Maven complains if a class with JUnit 4 annotations has methods whose name matches ".*test.*", and breaks the compile. That was pretty useful to locate methods that the regexes missed. It also meant changing test methods that were disabled by adding a prefix to "test" (e.g.. xtestMyCase), and annotating them with both @Test and @Ignore. - There was still a risk of having less tests after the migration, so I've made a sequential build both on the current master and in the branch, and compared. Found I missed a few test classes and added them back. The final result is that the build runs a few more tests than the current master, as there were test methods disabled that were actually passing! So all of this is great eh? Hem... nope, not quite as I hoped at least. My last step was to add PMD checks to ensure no new JUnit3 tests could be added. Created a custom PMD rule for it, based on type matching (sort of a XPath match), and found I had to disable it for OnlineTest. Nice! But then I noticed it would not fail in imagemosaicjdbc, where there is a separate base class for online test (why it could not use OnlineTest, it's beyond me)... why oh why? And then it dawned on me.... PMD is actually not running on test sources! It needs a dedicated flag <https://maven.apache.org/plugins/maven-pmd-plugin/pmd-mojo.html#includeTests> to do so. So, all the test sources, they have never been gone though the PMD checks. First, I've tried to set-up the maven build so that different checks are run on main sources and tests... and failed. The attempt tried to use two different executions, linked to different phases, giving each a different PMD configuration, and found it was ignoring my config, solid.... this is due to an issue in the PMD plugin, it only honors the configuration under plugin, ignoring the eventual one in executions: https://issues.apache.org/jira/browse/MPMD-132 Long story short, the above PR does not contain any new PMD check, as they do nothing on tests anyways. Plan to move forward: - Complete the migration on GWC and GS (should be much quicker, most GeoServer tests are JUnit 4 already). - Go back, and enable PMD on tests as well. That's gonna be epic too, gt-main alone contains a few hundred failures... but oh well, at this point I've become decently good at resolving them :-D - Finally, take a stab at migrating the OnlineTest family as well. Cheers Andrea == 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.*
_______________________________________________ GeoTools-Devel mailing list GeoTools-Devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/geotools-devel
