Core: enabling context-sensitive tests to start modules

Development
, ,
21

Would be easier for me if this was a pull request.

22

Thanks @dkayiwa, itā€™s here.

What about using Git tho?

git remote add mks https://github.com/mekomsolutions/openmrs-core
git fetch mks
git checkout mks/TRUNK-5213

Then you can ask Git for the differences between master and mks/TRUNK-5213, or use whatever else Git tool that you like to analyse commits.

23

Anyway deep down I think that I understand what the issue is, or as least the nature of the problem.

The context is made of a sort of merge of multiple contexts, the last one that is ā€œaddedā€ (ctx in StartModuleExecutionListener, here) is as expected after being refreshed. But the issue is that the bean that was initially added to its parent context is still there, making the test failing in the end.

24

Can you add this to the documentation? :slight_smile:

25

Mmm sure but what part exactly?

26

The git tip about the alternative to using a pull request.

27

Here is where I am right now:

Marking the context as dirty in StartModuleExecutionListener produces the desired effect, the bean definition goes away. However as soon as I do that I get this error upon refreshing the context:

Another CacheManager with same name 'hibernateCache' already exists in the same VM. Please provide unique names for each CacheManager in the config or do one of following:
1. Use one of the CacheManager.create() static factory methods to reuse same CacheManager with same name or create one if necessary
2. Shutdown the earlier cacheManager before creating new one with same name.
The source of the existing CacheManager is: DefaultConfigurationSource [ ehcache.xml or ehcache-failsafe.xml ]

Unlike apiCache there is no ad-hoc config for hibernateCache that says that it can be reused (see here for apiCache).

Not sure if Iā€™m looking in the right direction, but if yes, thatā€™s where Iā€™m blocked now.

28

As Springā€™s Sam Brannen says himself:

By design, programmatic refreshing of an ApplicationContext is not explicitly supported by the Spring TestContext Framework. Furthermore, it is not intended that a test method refresh a context.

Resource: ā€˜Reload or refresh a Spring application context inside a test method?ā€™.

This means that we face the following events in the scenario of a conditional bean loading based on a missing module:

  1. The Spring context is refreshed and the conditional bean definition is loaded, because at that point the module is missing.
  2. Later the module is started with @StartModule. Since the context canā€™t be refreshed again, the bean definition remains.

However bean definitions can be removed from an application context, and thatā€™s what I did here (based on a few assumptions).

@dkayiwa, everything works fine but I am still stuck with one weird thing: my new test OpenmrsProfileExcludeFilterWithModulesTest now passes fine in isolation:

mvn -Dtest=OpenmrsProfileExcludeFilterWithModulesTest test

But fails when I launch the entire build. I just spent an hour on this and couldnā€™t figure out why. Would you mind having a look on the PR that I made?

29

@dkayiwa I have narrowed this down (on Oracle JDK) to the collision between two tests. And those two tests involve starting modules:

I have updated my temp PR for TRUNK-5213. You will see that @Ignore-ing either of those two tests let the whole build pass. However as soon as those two tests are run together the build breaks on either one of them, depending on which one is run first.

Would you mind taking a look and sharing your findings?

30

It turns out that there is a bug with @StartModule, I created a new ticket for it: TRUNK-5216: ā€™@StartModule cannot be used with more than one test caseā€™.

Iā€™m happy to look into it but I would need some pointers as I havenā€™t quite figured out why/how tests that start modules may collide.

Cc: @dkayiwa, @darius, @wyclif

31

That test passes very nicely for me.

32

To double check I started from fresh environments on two systems (Ubuntu and macOS) where I cleared .m2 and cloned the Core again. Although the builds fail differently in each environment because - I suppose - tests are being run in different orders breaking the build differently, they both fail.

I have made a bug PR (here) where you can see the Linux stack trace left by TravisCI:

java.lang.IllegalArgumentException: interface org.openmrs.module.test1.api.Test1Service is not visible from class loader

33

@mksd i have just made a commit for TRUNK-5216 Pull these in and test your changes again to give us feedback.

1 Like
34

Thanks @dkayiwa! I will do that and let you know.

35

this seemed problematic with mocking static methods such as for utilities, i however found org.jmockit the way out