[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [mdt-bpmn2.dev] Setting up a test environment for the merger tool (was: Working on the ecore merger tool)

Hi,

I have filed a bug in Bugzilla
(https://bugs.eclipse.org/bugs/show_bug.cgi?id=377201). It will set up
the project as we discussed.

Now that we have this done, it's finally time to get our hands on the
code ;-) There are several things that can be changed to make testing
easier. Here is what I think of:

1. put cmof and xsd files into git repository - why do we have to
provide the files to the merger tool? we can easily put them under
src/main/resources.
2. Pass mandatory arguments to the processors constructor rather than
to loadAll - A processor instance can only be set into a usable state,
if all models are created. Having a loadAll method makes it possible
to call process() without having the processor in a valid state for
processing. Mandatory parameters should be passed to a constructor (or
the Facotry Method / Builder Pattern can be used).
3. Inject models into the merger, rather than string arguments -
dependecy injection is one key to testability. It is much easier to
test the processor, if some entity creates the models and injects them
into the processor rather than having the processor create the models
on its own.
4. Make Model a static class (this is required for step 4) - As long
as model is no static class, instances can only be created using a
processor instance. This is the case because Models add themselves to
the resourceset of the processor instance. I do not understand why we
need the resourceset. There are no methods in processor, that use it
(aside from loadAll, where the BPMN exceptions are added to the
resourceset). If there is a reason for having the resourceset - maybe
some EMF magic that relies on the resourceset - we should add a method
that provides access to a processors resourceset, so that the models
can be added to it.

Regards,
Benedikt

Am 15. April 2012 23:46 schrieb Benedikt Ritter <beneritter@xxxxxxxxxxxxxx>:
> Hey Reiner,
>
> thanks for the info.
> I've found a very nice solution that works here:
> http://www.java-forum.org/deployment/116917-tycho-unit-tests.html
> It's in german so I'll sum it up for the ML:
> The merger tool does not need OSGi runtime, so we configure the build
> to use the normal maven compiler (and not the tycho compiler). This
> way we can use the maven project layout where test classes are located
> in src/test/java and the project can be packaged as eclipse-plugin. In
> addition to that, the surefire plugin has to be configured
> accordingly.
> The JUnit dependency get's managed by maven and therefore has not to
> be added to the MANIFEST. Developers have to tell their IDE to use
> JUnit sources (simply by adding JUnit as a library to to classpath).
> This way calling 'mvn test' will execute the junit tests.
>
> Now I have a question about the parent pom. At first I added the
> merger tool as a new module to bpmn parent pom, but that does not feel
> right. We should not blow up the parent pom with that stuff. OTOH I
> would have to duplicate a lot of the configuration from the parent pom
> to the merger's pom in order to get everything up and running.
> What do you think about this?
>
> Regards,
> Benedikt
>
>
> Am 14. April 2012 20:02 schrieb Hille-Doering, Reiner
> <reiner.hille-doering@xxxxxxx>:
>> ... or you make the whole Merger including test a single eclipse-test-plugin. It would have a mandatory dependency to a JUnit bundle in its MANIFEST, which is not critical, as we would anyway not ship the Merger. Tycho will consider all source directories that are declared in build.properties - so you could have an extra one for the tests. Tycho-surefire will find and execute all JUnt3 and JUnit4 tests automatically in the integration test phase (therefore a Mvn install might be required).
>>
>> Am 14.04.2012 um 19:00 schrieb "Hille-Doering, Reiner" <reiner.hille-doering@xxxxxxx>:
>>
>>> Benedikt,
>>> the. simple rule is: For normal Java projects you can add the tests directly into the project. For PDE/bundle projects however, you should create an extra project that forms a fragment of the tested bundle.
>>> It is correct that the Merger is a normal project, however it could be tricky to configure the dependency to EMF etc without it also becoming a bundle. Note that we didn't have a Maven build for the Merger so far.
>>> I suggest to make the merger a Bundle project (Maven type eclipse-plugin) and a separate test fragment (Maven type eclipse-test-plugin).
>>>
>>> Reiner.
>>>
>>> Am 13.04.2012 um 11:38 schrieb "Benedikt Ritter" <beneritter@xxxxxxxxxxxxxx>:
>>>
>>>> Hey,
>>>>
>>>> I've tried to set up a test environment using the maven project
>>>> layout. I did the following:
>>>>
>>>> - Add JUnit 4.8.1 Bundle as plugin dependency
>>>> - set up new source folders src/main/java and src/test/java
>>>> - add a pom.xml that inherits from the bpmn2 parent pom
>>>> - add ecoremerger as module to the parent
>>>>
>>>> Running unit tests from within the IDE works fine. How ever, if I call
>>>> mvn test on the project the tests will not be executed. I don't know
>>>> tycho to well but I suspect that having the test sources in the same
>>>> project does not work out. I guess one would have to create a new
>>>> project and declare packaging as eclipse-test-plugin. Does anybody
>>>> know how to set up tycho to execute tests that reside in the same
>>>> project? I can upload what I've done so far to Bugzilla, if you want.
>>>>
>>>> ---
>>>>
>>>> Taking one step back, it looks odd to me, that the ecore merger tool
>>>> is a PDE project. The merger tool is a plain java app, so it would be
>>>> nice to have it set up that way. I guess the problem is, that the
>>>> project depends on EMF and that EMF is not been published to a maven
>>>> repo. Maybe we can find a way to convert the project to a normal maven
>>>> project and get the EMF dependencies on some other way?
>>>>
>>>> Beside that I've seen unused dependencies to:
>>>> org.eclipse.jface
>>>> org.eclipse.ui
>>>>
>>>> I guess we can drop those?
>>>>
>>>> Regards from the rainy rhine area
>>>> Benedikt
>>>>
>>>> Am 10. April 2012 11:42 schrieb Benedikt Ritter <beneritter@xxxxxxxxxxxxxx>:
>>>>> Am 10. April 2012 11:32 schrieb Hille-Doering, Reiner
>>>>> <reiner.hille-doering@xxxxxxx>:
>>>>>> Hi Benedikt,
>>>>>> again, I see no conceptual problem for the mentioned bug: My idea for this and similar cases is to modify/extend the merge tool that it can work on the current Ecore model without touching any manually done changes.
>>>>>
>>>>> Now I get it! Sorry ;-)
>>>>>
>>>>>> The "old" merge tool would regenerate the whole Ecore from scratch, thus overwriting all manual changes.
>>>>>> The "new" would open the checked-in Ecore model and would only apply specific changes to it. In case of Bug 357088 is would walk all classes of the Ecore, walk the XSD is parallel and add a special new annotation - call it "XSD_Index". To make is even more robust, the tool could check if the Annotation already exist and warn or ignore it in this case.
>>>>>>
>>>>>> Makes sense?
>>>>>>
>>>>>
>>>>> Sounds good to me. I'll create a patch that adds JUnit to the project
>>>>> as soon as possible.
>>>>>
>>>>>> Reiner.
>>>>>>
>>>>>>
>>>>>>
>>>>>> -----Original Message-----
>>>>>> From: mdt-bpmn2.dev-bounces@xxxxxxxxxxx [mailto:mdt-bpmn2.dev-bounces@xxxxxxxxxxx] On Behalf Of Benedikt Ritter
>>>>>> Sent: Dienstag, 10. April 2012 10:48
>>>>>> To: BPMN2 Developers Mailing List
>>>>>> Subject: Re: [mdt-bpmn2.dev] Working on the ecore merger tool
>>>>>>
>>>>>> Hey guys,
>>>>>>
>>>>>> the problem I'm seeing for example with Bug 357088 - XML
>>>>>> Serialization: incorrect order of children
>>>>>> (https://bugs.eclipse.org/bugs/show_bug.cgi?id=357088) is, that the
>>>>>> ecore model has been modified manually. If we fix the bug by modifying
>>>>>> the merger tool and re-generating the ecore model, how can we be sure
>>>>>> that we don't forget to apply one of those manual changes?
>>>>>>
>>>>>> So if we agree work on the merger, we have to identify those changes.
>>>>>> Then the merger has to be modified so that it can apply those changes
>>>>>> automatically. Otherwise every bug fix on the merger level would mean
>>>>>> to do a modification iteration on the ecore.
>>>>>>
>>>>>> I'm getting the feeling, that this merger tool discussion is blocking
>>>>>> the hole development on the project. So a decision should be made
>>>>>> ASAP.
>>>>>>
>>>>>> Regards,
>>>>>> Benedikt
>>>>>>
>>>>>>
>>>>>> Am 2. April 2012 17:31 schrieb Henning Heitkötter <hheitkoetter@xxxxxxxxxxxxxx>:
>>>>>>> Hi Benedikt, hi Reiner!
>>>>>>>
>>>>>>> Benedikt, sorry for the confusion, so let me explain:
>>>>>>> The initial version of the Ecore file of BPMN2 was created with the
>>>>>>> merger tool, because XSD and CMOF differ in quite a lot of aspects
>>>>>>> (e.g. naming and type of references). So, having both versions merged
>>>>>>> automatically was pretty much necessary.
>>>>>>> Changes that were necessary afterwards have exclusively been applied
>>>>>>> to the generated Ecore file. Hence, up to now, we have always pursued
>>>>>>> the second strategy mentioned by Benedikt.
>>>>>>>
>>>>>>> Based on this situation, I would propose to continue with this
>>>>>>> strategy. I think that all bugs mentioned could also be tackled that
>>>>>>> way. While I certainly see the benefits of the merger tool, I
>>>>>>> personally assume it to be too much work maintaining it to match the
>>>>>>> current state when the same can also be achieved on basis of the Ecore
>>>>>>> file.
>>>>>>> However, if you/we decide to go ahead with the merger tool anyway, I
>>>>>>> would certainly support and use the other strategy (apply changes to
>>>>>>> the merger instead of the Ecore file) afterwards, even though - as
>>>>>>> explained - I would not propose and promote this option myself due to
>>>>>>> the initial investment necessary.
>>>>>>>
>>>>>>> To sum it up, my proposal would be to continue with our current
>>>>>>> strategy, as long as no one steps up and adapts the merger tool to the
>>>>>>> current state.
>>>>>>>
>>>>>>> Regards,
>>>>>>> Henning
>>>>>>>
>>>>>>> PS: not to complicate things, but just to mention it - one can also
>>>>>>> use model transformation tools like QVTO to apply mass changes to the
>>>>>>> Ecore file. For example, I've used this approach to fix bug 373502.
>>>>>>>
>>>>>>> 2012/3/30 Hille-Doering, Reiner <reiner.hille-doering@xxxxxxx>:
>>>>>>>> Hi Benedikt, hi Henning,
>>>>>>>> all exchanged information is correct. However, I see a lot of value in Benedikt's idea to invest in the merge tool for the following reasons:
>>>>>>>> 1. There is not always a complete regeneration needed. E.g for the mentioned Bug my idea was to run the merger and let it just add a new custom attribute to the existing model. For those tasks it also helps to modularize the merger.
>>>>>>>> 2. For the more unlikely case that a full regernation is neede, e.g. for BPMN 2.1, a working strategy would be a regeration to a new model and then using EMF compare to manually merge in the new classes.
>>>>>>>>
>>>>>>>> Reiner.
>>>>>>>>
>>>>>>>> Am 30.03.2012 um 15:42 schrieb "Henning Heitkötter" <hheitkoetter@xxxxxxxxxxxxxx>:
>>>>>>>>
>>>>>>>>> So far, I (and we as a project) have neglected the maintenance of the
>>>>>>>>> merger tool in favor of changing the Ecore file directly, because the
>>>>>>>>> latter is easier to do and requires only one generation step, instead
>>>>>>>>> of two (Merger + Genmodel).
>>>>>>>>>
>>>>>>>>> What would be preferable with respect to new releases of the
>>>>>>>>> specification depends on the kind and amount of changes in these
>>>>>>>>> releases :-). However, if there were too many changes to apply them
>>>>>>>>> manually, we could still implement a new merge tool (based on the
>>>>>>>>> existing one), which could also incorporate our manual changes.
>>>>>>>>> So, in my opinion, we should dedicate our limited resources to other
>>>>>>>>> areas first.
>>>>>>>>>
>>>>>>>>> One thing to keep in mind if we decided to update the merger anyway /
>>>>>>>>> sometime: the genmodel file was also modified manually. Hence, we
>>>>>>>>> would have to check, which of these changes affected individual
>>>>>>>>> elements and how to align them with the merger.
>>>>>>>>>
>>>>>>>>> Regards,
>>>>>>>>> Henning
>>>>>>>>>
>>>>>>>>> 2012/3/30 Benedikt Ritter <beneritter@xxxxxxxxxxxxxx>:
>>>>>>>>>> Hey,
>>>>>>>>>>
>>>>>>>>>> the general question is, where are you going with the merger tool? Do
>>>>>>>>>> you want to use it for subsequent releases of the spec? Then the
>>>>>>>>>> project has to be in a maintainable state, with unit tests,
>>>>>>>>>> checkstyle, findbugs, etc.
>>>>>>>>>> Special configuration issues as mentioned by Henning should be
>>>>>>>>>> extracted from the code to some sort of config file (as far as
>>>>>>>>>> possible).
>>>>>>>>>>
>>>>>>>>>> If on the other hand, migrating the ecore by hand to a new spec
>>>>>>>>>> release is far less effort then re-generating it and adjusting the
>>>>>>>>>> generated model, it should be considered to drop the merge tool
>>>>>>>>>> completely.
>>>>>>>>>>
>>>>>>>>>> If you see a chance for bringing the tool up to date, then I'll start
>>>>>>>>>> right away, by writing some tests.
>>>>>>>>>>
>>>>>>>>>> Benedikt
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Am 30. März 2012 14:28 schrieb Henning Heitkötter <hheitkoetter@xxxxxxxxxxxxxx>:
>>>>>>>>>>> Hi all,
>>>>>>>>>>>
>>>>>>>>>>> I recently had a look at the merger tool and generated a merged Ecore
>>>>>>>>>>> file for testing purposes. I found that our ecore file has diverged
>>>>>>>>>>> from the automatically generated file in a lot of areas. (Of course,
>>>>>>>>>>> you can also look at the git log for the file to see these changes.)
>>>>>>>>>>> I also had some problems with attribute types (the ecore file that I
>>>>>>>>>>> had generated from the CMOF file only used EJavaObject), but that
>>>>>>>>>>> might have been due to some configuration issues on my side.
>>>>>>>>>>>
>>>>>>>>>>> While I think that bringing the merger tool up-to-date with these
>>>>>>>>>>> changes might be useful for future changes to the specification, be
>>>>>>>>>>> advised that it will likely require a lot of work and manual
>>>>>>>>>>> modification of the generated file (or special rules) might still be
>>>>>>>>>>> necessary, as not all changes can be deducted from the specification
>>>>>>>>>>> (CMOF + XSD) alone.
>>>>>>>>>>>
>>>>>>>>>>> Regards,
>>>>>>>>>>> Henning
>>>>>>>>>>>
>>>>>>>>>>> 2012/3/29 Hille-Doering, Reiner <reiner.hille-doering@xxxxxxx>:
>>>>>>>>>>>> Hi Benedikt,
>>>>>>>>>>>> I'm fine with the idea. Feel free to reorganize the project to fit to Maven convention structure. Only note that for bundles (for which we use tycho), the structure is different and more PDE like - and bundle tests go to extra bundles or fragments.
>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Regards,
>>>>>>>>>>>> Reiner.
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Am 29.03.2012 um 22:40 schrieb "Benedikt Ritter" <beneritter@xxxxxxxxxxxxxx>:
>>>>>>>>>>>>
>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>
>>>>>>>>>>>>> I'm planning to put some effort on the ecore merger tool, since there
>>>>>>>>>>>>> are several issues related with that tool (e.g.
>>>>>>>>>>>>> https://bugs.eclipse.org/bugs/show_bug.cgi?id=357088 or the discussed
>>>>>>>>>>>>> splitting of the ecore model). However, I'm feeling uncomfortable,
>>>>>>>>>>>>> touching the code because there are no unit tests. So the first thing
>>>>>>>>>>>>> I want to do is to write some unit tests.
>>>>>>>>>>>>> Now at my company we use maven. Maven has a convention for the project
>>>>>>>>>>>>> structure, where unit tests reside in special source folder called
>>>>>>>>>>>>> src/test/java. Since the ecore merger tool does not stick to this
>>>>>>>>>>>>> convention, I do not know where to put my unit tests. I've seen the
>>>>>>>>>>>>> test bundle but I don't think that it is the right place for the ecore
>>>>>>>>>>>>> merger unit tests. So where should I put the tests?
>>>>>>>>>>>>>
>>>>>>>>>>>>> After bulding up a basic test coverage (and ATM I don't know how much
>>>>>>>>>>>>> effort that will be), I'd like to try to split the code up some more.
>>>>>>>>>>>>> The processor has several concerns that it has to deal with, and it
>>>>>>>>>>>>> would be good to separate concerns.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Regards,
>>>>>>>>>>>>> Benedikt
>>>>>>>>>>>>> _______________________________________________
>>>>>>>>>>>>> mdt-bpmn2.dev mailing list
>>>>>>>>>>>>> mdt-bpmn2.dev@xxxxxxxxxxx
>>>>>>>>>>>>> https://dev.eclipse.org/mailman/listinfo/mdt-bpmn2.dev
>>>>>>>>>>>> _______________________________________________
>>>>>>>>>>>> mdt-bpmn2.dev mailing list
>>>>>>>>>>>> mdt-bpmn2.dev@xxxxxxxxxxx
>>>>>>>>>>>> https://dev.eclipse.org/mailman/listinfo/mdt-bpmn2.dev
>>>>>>>>>>> _______________________________________________
>>>>>>>>>>> mdt-bpmn2.dev mailing list
>>>>>>>>>>> mdt-bpmn2.dev@xxxxxxxxxxx
>>>>>>>>>>> https://dev.eclipse.org/mailman/listinfo/mdt-bpmn2.dev
>>>>>>>>>> _______________________________________________
>>>>>>>>>> mdt-bpmn2.dev mailing list
>>>>>>>>>> mdt-bpmn2.dev@xxxxxxxxxxx
>>>>>>>>>> https://dev.eclipse.org/mailman/listinfo/mdt-bpmn2.dev
>>>>>>>>> _______________________________________________
>>>>>>>>> mdt-bpmn2.dev mailing list
>>>>>>>>> mdt-bpmn2.dev@xxxxxxxxxxx
>>>>>>>>> https://dev.eclipse.org/mailman/listinfo/mdt-bpmn2.dev
>>>>>>>> _______________________________________________
>>>>>>>> mdt-bpmn2.dev mailing list
>>>>>>>> mdt-bpmn2.dev@xxxxxxxxxxx
>>>>>>>> https://dev.eclipse.org/mailman/listinfo/mdt-bpmn2.dev
>>>>>>> _______________________________________________
>>>>>>> mdt-bpmn2.dev mailing list
>>>>>>> mdt-bpmn2.dev@xxxxxxxxxxx
>>>>>>> https://dev.eclipse.org/mailman/listinfo/mdt-bpmn2.dev
>>>>>> _______________________________________________
>>>>>> mdt-bpmn2.dev mailing list
>>>>>> mdt-bpmn2.dev@xxxxxxxxxxx
>>>>>> https://dev.eclipse.org/mailman/listinfo/mdt-bpmn2.dev
>>>>>> _______________________________________________
>>>>>> mdt-bpmn2.dev mailing list
>>>>>> mdt-bpmn2.dev@xxxxxxxxxxx
>>>>>> https://dev.eclipse.org/mailman/listinfo/mdt-bpmn2.dev
>>>> _______________________________________________
>>>> mdt-bpmn2.dev mailing list
>>>> mdt-bpmn2.dev@xxxxxxxxxxx
>>>> https://dev.eclipse.org/mailman/listinfo/mdt-bpmn2.dev
>>> _______________________________________________
>>> mdt-bpmn2.dev mailing list
>>> mdt-bpmn2.dev@xxxxxxxxxxx
>>> https://dev.eclipse.org/mailman/listinfo/mdt-bpmn2.dev
>> _______________________________________________
>> mdt-bpmn2.dev mailing list
>> mdt-bpmn2.dev@xxxxxxxxxxx
>> https://dev.eclipse.org/mailman/listinfo/mdt-bpmn2.dev