Would be easier for me if this was a pull request.
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.
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” (
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.
Can you add this to the documentation?
Mmm sure but what part exactly?
The git tip about the alternative to using a pull request.
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 ]
apiCache there is no ad-hoc config for
hibernateCache that says that it can be reused (see here for
Not sure if I’m looking in the right direction, but if yes, that’s where I’m blocked now.
As Spring’s Sam Brannen says himself:
By design, programmatic refreshing of an
ApplicationContextis not explicitly supported by the Spring TestContext Framework. Furthermore, it is not intended that a test method refresh a context.
This means that we face the following events in the scenario of a conditional bean loading based on a missing module:
- The Spring context is refreshed and the conditional bean definition is loaded, because at that point the module is missing.
- 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?
@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?
It turns out that there is a bug with
@StartModule, I created a new ticket for it:
@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.
That test passes very nicely for me.
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
@mksd i have just made a commit for TRUNK-5216 Pull these in and test your changes again to give us feedback.