vitruv-tools / Vitruv

View-based Development and Model Consistency Framework
http://vitruv.tools
Eclipse Public License 1.0
14 stars 19 forks source link

Potential bug inside ModelDeepEqualityMatcher.xtend #494

Closed NeumannDirk closed 2 years ago

NeumannDirk commented 2 years ago

Working on the #477 I found the following oddity: The FamilyRegister.families is an unordered EReference (see here https://github.com/kit-sdq/DemoMetamodels/blob/master/bundles/edu.kit.ipd.sdq.metamodels.families/model/families.ecore#L5). Therefore, I would expect that comparing two FamilyRegister - which each contain a list of equal families in different orders - will pass a test using an equalsDeeply-Matcher.

This seems not to be the case, since the test attached to this issue fails with the error message which is pasted below the test code: testDeeplyEqualsMatcher.txt

A different test from the current #477 returns a similar error which is also attached here: maybeBugInDeepEqualityMatcher.txt

The error messages themselves already show that the expected FamilyRegister and the actual FamilyRegister are equal to each other (at least from my understanding of equality). It seems that this bug originates from these lines in the code: https://github.com/vitruv-tools/Vitruv/blob/master/bundles/testutils/tools.vitruv.testutils/src/tools/vitruv/testutils/matchers/ModelDeepEqualityMatcher.xtend#L87-L90 Unfortunately I was not able to locate the origin of this behaviour any further.

HeikoKlare commented 2 years ago

The referred lines in the ModelDeepEqualityMatcher call EMF Compare to compare the two elements, in the example the two FamilyRegisters. So the bug is not related to Vitruv or the mathcer implementation. However, the bug that the unordered list is considered as ordered is also not a bug of EMF Compare but is actually a limitation of core EMF: EMF generates the same code for multi-valued attributes and references no matters whether ordered is set or not (see also page 108, section 5.3 in Steinberg's EMF book or the EMF implementation). They are always realized by a simple list behaving like an ordered multi-valued reference. So setting ordered in Ecore models does actually have not effect.

In consequence, despite providing a fixed code generator for EMF (which may break compatibility with EMF-based tools), we cannot do anything to fix this bug. If I miss anything, please let me know. Otherwise I would propose to close this issue.

With respect to #477, this bug should not a problem as simply considering the reference as ordered should "solve" (or more precisely circumvent) the problem. We may also change the families metamodel and make the reference (and potentially also other references) ordered, as setting them unordered has currently no effect and may, as this issue shows, rather be misleading.

NeumannDirk commented 2 years ago

Thank you for the explanations. If that is the case I will just add the objects to the reference in the order needed to pass the test and keep this behaviour in mind. Therefore, I think this issue can be closed. Thanks.