SORMAS-Project icon indicating copy to clipboard operation
SORMAS-Project copied to clipboard

Replace CDI unit testing framework

Open MartinWahnschaffe opened this issue 3 years ago • 2 comments

Problem Description

Unit testing of the SORMAS backend is currently facing multiple problems:

  • Bean-test no longer maintained (e.g. no support for jakarta-ee, using very old versions of weld + deltaspike) -> we would have to fork and maintain on our own
  • Combination of bean-test and mocking is not working very good. E.g. it's tricky to change the logged-in user during test run and hard to mock configuration properties, feature configurations
  • Transaction logic is partly not testable (e.g. annotation TransactionAttribute)
  • Validation annotations are not testable (e.g. #7611)
  • Parts of logic data stored in JSON format can't be tested (e.g. properties of feature configurations). -> This will not be addressed here. Maybe an update of H2 can also fix this problem.
  • Is overall a bit slow, because we are setting up the complete backend CDI container for each test class

Possible Test Approaches

Generally speaking there are three approaches to testing backend functionality:

  1. Basic unit tests + mocking
  • Mocking all the dependencies of tested services and facades would be a lot of work and make the tests a lot less helpful
  1. Integration tests (e.g. based on Arquillian)
  • Every test run requires an archive to be built and deployed which makes the testing slower. Too slow if you need to run thousands of test cases.
  • Creating the deployments, especially resolving the required artifacts and classes, is not trivial.
  • Setup tends to be complicated.
  • Still helpful for functionality that can't be tested otherwise (e.g. transaction based stuff or database specific stuff)
  1. CDI-based testing (provides bean container that automatically injects dependencies)
  • This is the approach we have followed so far and should go on using.
  • It's currently facing the problems mentioned above, though

Proposed Change

  • [ ] Find a replacement for the novatec bean-test framework.
  • cdi-test: Small open source project, but maintained, using a current version of weld and is not relying on deltaspike (-> faster) It's supposed to make mocking of parts of the system a lot easier
  • cdi-unit: http://cdi-unit.github.io/cdi-unit/
  • weld-testing: https://dzone.com/articles/weld-junit-easy-testing-of-cdi-beans
  • [ ] Check whether both can be used in parallel to simplify migration
  • [ ] Make transaction logic testable, e.g. provide an interceptor for TransactionAttribute annotation
  • [ ] Make validation testable
  • [ ] Create implementation/migration issue(s)

Additional Information

Alternative approach:Use Spring instead of JavaEE for the backend. This would probably be a very big effort. If we are considering this it would have to be evaluated.

https://cdi-test.hilling.de/ http://www.mastertheboss.com/java-ee/jakarta-ee/testing-jakarta-ee-applications-with-cdi-test/ https://entwickler.de/container/qualitatssicherung-im-container-leichtgewichtiges-testen-von-cdi-komponenten https://www.sigs-datacom.de/uploads/tx_dmjournals/barragan_wanner_baier_JS_05_13_kjgu.pdf https://antoniogoncalves.org/2018/01/16/java-ee-vs-spring-testing/ https://itnext.io/testing-jakarta-ee-9-applications-with-arquillian-and-payara-6-52fd153d8d9 https://medium.com/riag-digital-techblog/why-you-should-migrate-to-junit-5-15438f5c56f

MartinWahnschaffe avatar Aug 08 '22 13:08 MartinWahnschaffe

Note: While creating this issue I had to do a few first steps to see what's possible in general. I have already tried moving one of the test classes from bean-test to cdi-test, which worked quite good. Unfortunately it's not working with bean-test in parallel out of the box, because the TransactionalInterceptor of bean test is somehow not executed. This should be fixable, though, but we should probably also have a look at cdi-unit and weld-testing, because both seem to have a bigger user group.

MartinWahnschaffe avatar Aug 08 '22 14:08 MartinWahnschaffe

Show me as +1 for Spring in the midterm future :D

JonasCir avatar Aug 10 '22 09:08 JonasCir

I have created a POC branch that replaces bean-test with cdi-test: https://github.com/hzi-braunschweig/SORMAS-Project/tree/10074-poc-cdi-test

So far it works fine. It's now possible to directly Inject services into test classes as-well. What is missing for cdi-test is:

  • Reset EntityManger after each test
  • Check whether annotation based validation in DTOs is testable. See #7611
  • Check how to test TransactionAttribute. Should probably work with an interceptor and is from my point of view not related to the question which test library we are using.

Furthermore I feel that weld-testing is also worth a POC. It seems to be a bit more sophisticated, giving us more influence on which beans are actually loaded and thus likely allowing us to speed up test execution.

For both libraries I'd strongly suggest to upgrade our unit tests to Junit5. For cdi-test it's a pre-requisite, for weld-testing it seems to be better documented and more convenient to use JUnit5.

MartinWahnschaffe avatar Oct 28 '22 08:10 MartinWahnschaffe

I agree with @MartinWahnschaffe that weld-testing should be looked into (has higher traffic and contribution than cdi-test).

StefanKock avatar Dec 21 '22 07:12 StefanKock

Unfortunately, we cannot use weld-testing in the current form. weld-testing bootstraps are pure CDI container without any EJB extension, which means that none of our beans will work. I did not found a simple to use portable EJB extension for CDI, if someone knows a library for this, I'm happy for a hint.

It shows that we need is a real Java EE container/execution environment out our disposal in the tests. I think we should use Arquillian for this, we also can use it for unit tests IMO. I just found this blogpost which looks interesting as you can inject your dependencies in your test and does not rely on a remote server. I also found this which might help.

In the next year, likely someone else needs to pick this up due my availability for this project.

JonasCir avatar Dec 30 '22 15:12 JonasCir

Ok I played around with Arquillian and the container starts, however, the deployment fails with the exception mentioned in this issue https://github.com/payara/Payara/issues/3278. I use payara version 5.2021.10 (the same version we use in production) and open JDK 11 which I think should be compatible but obviously isn't. Dunno :shrug:

JonasCir avatar Jan 02 '23 15:01 JonasCir

Unfortunately, we cannot use weld-testing in the current form. weld-testing bootstraps are pure CDI container without any EJB extension, which means that none of our beans will work. I did not found a simple to use portable EJB extension for CDI, if someone knows a library for this, I'm happy for a hint.

I tried the same on a demo project.

  1. Reading the documentation for weld-testing I got the impression that one has to do some effort to link non-CDI stuff.
  2. I also had somewhere the hint to add another test dependency org.jboss.weld.module:weld-ejb. But I stopped working just after my initial try that failed.

StefanKock avatar Jan 09 '23 08:01 StefanKock

Hey thanks for the pointer. Unfortunately, I was not able to connect the weld-ejb module with weld-testing. There is zero documentation and no examples for this. I tried to play around with the library but not luck. This ecosystem is insane...

JonasCir avatar Jan 09 '23 15:01 JonasCir

Meeting results with @StefanKock and @JonasCir:

  1. Go with hilling cdi-test. cdi-unit does not work with Jakarta. weld-testing does not support ejbs properly, so we would have to do it on our own.
  2. Probably do a complete replacement instead of parallel migration. With JUnit5 in place the changes in all affected classes should be minimal

Next step: Create updated POC based on the latest dev version. Focus on just replacing the existing library. Benefits of the new library can be looked into later.

MartinWahnschaffe avatar Jan 18 '23 08:01 MartinWahnschaffe

@StefanKock I have pushed my changes in this branch: https://github.com/hzi-braunschweig/SORMAS-Project/compare/change-10074-cdi-test-hilling

Done so far:

  • Replaced bean-test with cdi-test
  • Updated mockito to version used by cdi-test
  • CDI Container is only built once. Database is reset between tests using TestDatabaseCleaner (dropping the whole database)
  • Removed BaseBeanTest where used
  • Do history tables test without AbstractBeanTest overhead
  • Fixed TaskService unit tests (where working before, because user service logic was not properly working)

I can provide an html with the current status of the unit tests. Patterns that need to be fixed:

  • StackOverflow e.g. in testAggregateReportGetEditData related to fetching the current user. Seems to be related to the fact that the Transactional(RequiresNew) annotation of fetchUser is not working in the test. For bean test this worked, because AdoListener.getCurrentUser is doing lookup("java:global/sormas-ear/sormas-backend/CurrentUserService") which was mocked and did not provide a current user
  • NullPointerExceptions that is caused by the transaction not being opened and closed by each backend call from the test class and thus still having only partly initialized entities
  • Failed casts to SessionImpl (see my change in AdoServiceWithUserFilterTest)
  • S2S tests failing because weld failed to access AbstractSormasToSormasInterface$Persister

MartinWahnschaffe avatar Feb 06 '23 10:02 MartinWahnschaffe