Skip to main content
summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorLaurent Goubet2016-06-28 09:00:48 -0400
committerLaurent Goubet2016-07-12 10:49:53 -0400
commitd0eb2f08d3ee8ecff54d068a8747ccee5bd69ff3 (patch)
treea2af67873220c00540d5e028b9f5e93b66f0aa40
parent4a548a2ac74e3622bac8c0e08c2e5bcfa12de513 (diff)
downloadorg.eclipse.emf.compare-d0eb2f08d3ee8ecff54d068a8747ccee5bd69ff3.tar.gz
org.eclipse.emf.compare-d0eb2f08d3ee8ecff54d068a8747ccee5bd69ff3.tar.xz
org.eclipse.emf.compare-d0eb2f08d3ee8ecff54d068a8747ccee5bd69ff3.zip
Do not consider a container deletion to conflict with unsets under it
Since a containment deletion and the "unset"(s) under it were seen as a pseudo conflict with each other, we were "hiding" the deletion from view and preventing the merge altogether. This adds an explicit dependency between the core tests and objenesis to avoid failures when launching the build locally (mockito has a transient dependency to objenesis). Change-Id: I8d00c336285480962499d861575815f1c4b72b3d
-rw-r--r--plugins/org.eclipse.emf.compare.tests/META-INF/MANIFEST.MF3
-rw-r--r--plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/conflict/ConflictDetectionTest.java107
-rw-r--r--plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/conflict/PseudoConflictDetectionTest.java2
-rw-r--r--plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/edit/TestComparisonItemProviderSpec.java2
-rw-r--r--plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/fullcomparison/ExtLibraryTest.java27
-rw-r--r--plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/fullcomparison/IdentifierComparisonTest.java36
-rw-r--r--plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/fullcomparison/data/identifier/CHANGES6
-rw-r--r--plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/ConflictImplicationsTest_Bug484579.java66
-rw-r--r--plugins/org.eclipse.emf.compare/src/org/eclipse/emf/compare/conflict/DefaultConflictDetector.java15
-rw-r--r--plugins/org.eclipse.emf.compare/src/org/eclipse/emf/compare/internal/conflict/ContainmentRefChangeConflictSearch.java5
-rw-r--r--plugins/org.eclipse.emf.compare/src/org/eclipse/emf/compare/internal/conflict/ResourceAttachmentChangeConflictSearch.java5
-rw-r--r--plugins/org.eclipse.emf.compare/src/org/eclipse/emf/compare/merge/AttributeChangeMerger.java8
12 files changed, 177 insertions, 105 deletions
diff --git a/plugins/org.eclipse.emf.compare.tests/META-INF/MANIFEST.MF b/plugins/org.eclipse.emf.compare.tests/META-INF/MANIFEST.MF
index 2c092f71b..964cdc545 100644
--- a/plugins/org.eclipse.emf.compare.tests/META-INF/MANIFEST.MF
+++ b/plugins/org.eclipse.emf.compare.tests/META-INF/MANIFEST.MF
@@ -18,7 +18,8 @@ Require-Bundle: org.eclipse.core.runtime,
org.eclipse.emf.compare.edit,
org.eclipse.emf.transaction;bundle-version="1.3.0",
org.mockito;bundle-version="1.8.0",
- org.hamcrest;bundle-version="1.1.0"
+ org.hamcrest;bundle-version="1.1.0",
+ org.objenesis;bundle-version="1.0.0"
Bundle-Localization: plugin
Bundle-RequiredExecutionEnvironment: J2SE-1.5
Export-Package: org.eclipse.emf.compare.tests,
diff --git a/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/conflict/ConflictDetectionTest.java b/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/conflict/ConflictDetectionTest.java
index 66d269c48..15ac2c236 100644
--- a/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/conflict/ConflictDetectionTest.java
+++ b/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/conflict/ConflictDetectionTest.java
@@ -33,6 +33,7 @@ import static org.eclipse.emf.compare.utils.EMFComparePredicates.removedFromAttr
import static org.eclipse.emf.compare.utils.EMFComparePredicates.removedFromReference;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
@@ -264,9 +265,13 @@ public class ConflictDetectionTest {
final List<Diff> differences = comparison.getDifferences();
final List<Conflict> conflicts = comparison.getConflicts();
- // We should have no less and no more than 2 differences, composing a single conflict
+ // We should have 2 differences
+ // Deleted a container on one side, unset one of its attributes on the other
+ // There are no conflict on this since we do not consider pseudo conflicts between a container and its
+ // sub-diffs... and we do not consider attribute changes on the same side of the container deletion as
+ // standalone diffs (they're not even created as Diffs by the engine).
assertEquals(2, differences.size());
- assertEquals(1, conflicts.size());
+ assertTrue(conflicts.isEmpty());
final Predicate<? super Diff> leftDiffDescription = changedAttribute("root.conflictHolder",
"singleValuedAttribute", "origin", null);
@@ -279,15 +284,6 @@ public class ConflictDetectionTest {
assertNotNull(leftDiff);
assertNotNull(rightDiff);
-
- // We know there's only one conflict
- final Conflict conflict = conflicts.get(0);
-
- final List<Diff> conflictDiff = conflict.getDifferences();
- assertEquals(2, conflictDiff.size());
- assertTrue(conflictDiff.contains(leftDiff));
- assertTrue(conflictDiff.contains(rightDiff));
- assertSame(PSEUDO, conflict.getKind());
}
@Test
@@ -331,11 +327,12 @@ public class ConflictDetectionTest {
final Conflict conflict = conflicts.get(0);
final List<Diff> conflictDiff = conflict.getDifferences();
- assertEquals(3, conflictDiff.size());
+ assertEquals(2, conflictDiff.size());
assertTrue(conflictDiff.contains(leftReferenceDiff));
assertTrue(conflictDiff.contains(rightReferenceDiff));
- assertTrue(conflictDiff.contains(rightDeleteDiff));
assertSame(PSEUDO, conflict.getKind());
+
+ assertNull(rightDeleteDiff.getConflict());
}
@Test
@@ -955,7 +952,7 @@ public class ConflictDetectionTest {
// We should have no less and no more than 2 differences, composing a single conflict
assertEquals(2, differences.size());
- assertEquals(1, conflicts.size());
+ assertTrue(conflicts.isEmpty());
final Predicate<? super Diff> leftDiffDescription = removedFromAttribute("root.conflictHolder",
"multiValuedAttribute", "origin1");
@@ -968,15 +965,6 @@ public class ConflictDetectionTest {
assertNotNull(leftDiff);
assertNotNull(rightDiff);
-
- // We know there's only one conflict
- final Conflict conflict = conflicts.get(0);
-
- final List<Diff> conflictDiff = conflict.getDifferences();
- assertEquals(2, conflictDiff.size());
- assertTrue(conflictDiff.contains(leftDiff));
- assertTrue(conflictDiff.contains(rightDiff));
- assertSame(PSEUDO, conflict.getKind());
}
@Test
@@ -1032,11 +1020,12 @@ public class ConflictDetectionTest {
final Conflict conflict = conflicts.get(0);
final List<Diff> conflictDiff = conflict.getDifferences();
- assertEquals(3, conflictDiff.size());
+ assertEquals(2, conflictDiff.size());
assertTrue(conflictDiff.contains(leftReferenceDiff));
- assertTrue(conflictDiff.contains(rightDeleteDiff));
assertTrue(conflictDiff.contains(rightReferenceDiff1));
assertSame(PSEUDO, conflict.getKind());
+
+ assertNull(rightDeleteDiff.getConflict());
}
@Test
@@ -1129,11 +1118,11 @@ public class ConflictDetectionTest {
/*
* We expect 4 differences here. On the right side, an element has been deleted. On the left side, all
- * three values of one of this element's features have been removed. All three diffs on the left are
- * in conflict with the right diff.
+ * three values of one of this element's features have been removed. We do not consider this as a
+ * conflict.
*/
assertEquals(4, differences.size());
- assertEquals(1, conflicts.size());
+ assertTrue(conflicts.isEmpty());
final Predicate<? super Diff> leftAttributeDiff1Description = removedFromAttribute(
"root.conflictHolder", "multiValuedAttribute", "origin1");
@@ -1156,17 +1145,6 @@ public class ConflictDetectionTest {
assertNotNull(leftAttributeDiff2);
assertNotNull(leftAttributeDiff3);
assertNotNull(rightDiff);
-
- // We know there's only one conflict
- final Conflict conflict = conflicts.get(0);
-
- final List<Diff> conflictDiff = conflict.getDifferences();
- assertEquals(4, conflictDiff.size());
- assertTrue(conflictDiff.contains(leftAttributeDiff1));
- assertTrue(conflictDiff.contains(leftAttributeDiff2));
- assertTrue(conflictDiff.contains(leftAttributeDiff3));
- assertTrue(conflictDiff.contains(rightDiff));
- assertSame(PSEUDO, conflict.getKind());
}
@Test
@@ -1184,10 +1162,14 @@ public class ConflictDetectionTest {
/*
* We expect 7 differences here. On the right, an element has been deleted. All three values of this
* element's reference have been removed. On the left, we've also removed all three values of that
- * same reference. All 7 differences are in pseudo-conflict with each other.
+ * same reference.
+ */
+ /*
+ * The element deletion is a standalone diffs, but all of the diffs under it are in pseudo-conflict
+ * two-by-two.
*/
assertEquals(7, differences.size());
- assertEquals(1, conflicts.size());
+ assertEquals(3, conflicts.size());
final Predicate<? super Diff> referenceDiff1Description = removedFromReference("root.conflictHolder",
"multiValuedReference", "root.origin1");
@@ -1220,19 +1202,30 @@ public class ConflictDetectionTest {
assertNotNull(rightReferenceDiff2);
assertNotNull(rightReferenceDiff3);
- // We know there's only one conflict
- final Conflict conflict = conflicts.get(0);
+ // We're expecting three conflicts with 2 differences each
- final List<Diff> conflictDiff = conflict.getDifferences();
- assertEquals(7, conflictDiff.size());
- assertTrue(conflictDiff.contains(leftReferenceDiff1));
- assertTrue(conflictDiff.contains(leftReferenceDiff2));
- assertTrue(conflictDiff.contains(leftReferenceDiff3));
- assertTrue(conflictDiff.contains(rightDeleteDiff));
- assertTrue(conflictDiff.contains(rightReferenceDiff1));
- assertTrue(conflictDiff.contains(rightReferenceDiff2));
- assertTrue(conflictDiff.contains(rightReferenceDiff3));
- assertSame(PSEUDO, conflict.getKind());
+ final Conflict conflict1 = leftReferenceDiff1.getConflict();
+ assertNotNull(conflict1);
+ assertEquals(2, conflict1.getDifferences().size());
+ assertTrue(conflict1.getDifferences().contains(leftReferenceDiff1));
+ assertTrue(conflict1.getDifferences().contains(rightReferenceDiff1));
+ assertSame(PSEUDO, conflict1.getKind());
+
+ final Conflict conflict2 = leftReferenceDiff2.getConflict();
+ assertNotNull(conflict2);
+ assertEquals(2, conflict2.getDifferences().size());
+ assertTrue(conflict2.getDifferences().contains(leftReferenceDiff2));
+ assertTrue(conflict2.getDifferences().contains(rightReferenceDiff2));
+ assertSame(PSEUDO, conflict2.getKind());
+
+ final Conflict conflict3 = leftReferenceDiff3.getConflict();
+ assertNotNull(conflict3);
+ assertEquals(2, conflict3.getDifferences().size());
+ assertTrue(conflict3.getDifferences().contains(leftReferenceDiff3));
+ assertTrue(conflict3.getDifferences().contains(rightReferenceDiff3));
+ assertSame(PSEUDO, conflict3.getKind());
+
+ assertNull(rightDeleteDiff.getConflict());
}
@Test
@@ -2121,10 +2114,12 @@ public class ConflictDetectionTest {
final List<Conflict> conflicts = comparison.getConflicts();
assertEquals(5, differences.size());
- assertEquals(1, conflicts.size());
+ assertEquals(2, conflicts.size());
- Conflict soleConflict = conflicts.get(0);
- assertSame(PSEUDO, soleConflict.getKind());
+ for (Conflict conflict : conflicts) {
+ assertSame(PSEUDO, conflict.getKind());
+ assertEquals(2, conflict.getDifferences().size());
+ }
}
@Test
diff --git a/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/conflict/PseudoConflictDetectionTest.java b/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/conflict/PseudoConflictDetectionTest.java
index 94c7f3bdd..306a61ead 100644
--- a/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/conflict/PseudoConflictDetectionTest.java
+++ b/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/conflict/PseudoConflictDetectionTest.java
@@ -55,7 +55,7 @@ public class PseudoConflictDetectionTest {
*/
Object[] test1 = new Object[] {inputData.getPseudoConflictCase1Ancestor(),
inputData.getPseudoConflictCase1RightAndLeftModel(),
- inputData.getPseudoConflictCase1RightAndLeftModel(), new Integer(4) };
+ inputData.getPseudoConflictCase1RightAndLeftModel(), new Integer(5) };
/**
* Right and left models add the same univaluated containment EReference.
*/
diff --git a/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/edit/TestComparisonItemProviderSpec.java b/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/edit/TestComparisonItemProviderSpec.java
index a51c1e9c1..d4deea039 100644
--- a/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/edit/TestComparisonItemProviderSpec.java
+++ b/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/edit/TestComparisonItemProviderSpec.java
@@ -47,7 +47,7 @@ public class TestComparisonItemProviderSpec extends AbstractTestCompareItemProvi
Collection<?> children = itemProvider.getChildren(comparison);
- assertEquals(7, children.size());
+ assertEquals(9, children.size());
assertEquals(1, size(filter(children, Match.class)));
assertEquals(1, size(filter(children, MatchResource.class)));
}
diff --git a/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/fullcomparison/ExtLibraryTest.java b/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/fullcomparison/ExtLibraryTest.java
index fdbbdfbfa..048b1285a 100644
--- a/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/fullcomparison/ExtLibraryTest.java
+++ b/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/fullcomparison/ExtLibraryTest.java
@@ -89,16 +89,20 @@ public class ExtLibraryTest {
final Conflict readerConflict = rightRemovedReaderDiff.getConflict();
assertNotNull(readerConflict);
- assertEquals(4, readerConflict.getDifferences().size());
+ assertEquals(2, readerConflict.getDifferences().size());
assertTrue(readerConflict.getDifferences().contains(rightRemovedReaderDiff));
- assertTrue(readerConflict.getDifferences().contains(rightUnsetReaderTypeDiff));
assertTrue(readerConflict.getDifferences().contains(leftRemovedReaderDiff));
- assertTrue(readerConflict.getDifferences().contains(leftUnsetReaderTypeDiff));
- assertSame(readerConflict, rightUnsetReaderTypeDiff.getConflict());
assertSame(readerConflict, leftRemovedReaderDiff.getConflict());
- assertSame(readerConflict, leftUnsetReaderTypeDiff.getConflict());
assertSame(ConflictKind.PSEUDO, readerConflict.getKind());
+ final Conflict readerETypeConflict = rightUnsetReaderTypeDiff.getConflict();
+ assertNotNull(readerETypeConflict);
+ assertEquals(2, readerETypeConflict.getDifferences().size());
+ assertTrue(readerETypeConflict.getDifferences().contains(rightUnsetReaderTypeDiff));
+ assertTrue(readerETypeConflict.getDifferences().contains(leftUnsetReaderTypeDiff));
+ assertSame(readerETypeConflict, leftUnsetReaderTypeDiff.getConflict());
+ assertSame(ConflictKind.PSEUDO, readerETypeConflict.getKind());
+
final Predicate<? super Diff> rightRenamedFamilyname = and(fromSide(DifferenceSource.RIGHT),
changedAttribute("extlibrary.Person.familyName", "name", "lastName", "familyName"));
final Predicate<? super Diff> leftRemovedLastname = and(fromSide(DifferenceSource.LEFT),
@@ -153,13 +157,18 @@ public class ExtLibraryTest {
final Conflict titleConflict = leftRemovedTitleDiff.getConflict();
assertNotNull(titleConflict);
- assertEquals(4, titleConflict.getDifferences().size());
+ assertEquals(2, titleConflict.getDifferences().size());
assertTrue(titleConflict.getDifferences().contains(leftRemovedTitleDiff));
- assertTrue(titleConflict.getDifferences().contains(leftUnsetTitleTypeDiff));
assertTrue(titleConflict.getDifferences().contains(rightRemovedTitleDiff));
- assertTrue(titleConflict.getDifferences().contains(rightUnsetTitleTypeDiff));
assertSame(ConflictKind.PSEUDO, titleConflict.getKind());
- assertEquals(5, comparison.getConflicts().size());
+ final Conflict titleETypeConflict = leftUnsetTitleTypeDiff.getConflict();
+ assertNotNull(titleETypeConflict);
+ assertEquals(2, titleETypeConflict.getDifferences().size());
+ assertTrue(titleETypeConflict.getDifferences().contains(leftUnsetTitleTypeDiff));
+ assertTrue(titleETypeConflict.getDifferences().contains(rightUnsetTitleTypeDiff));
+ assertSame(ConflictKind.PSEUDO, titleETypeConflict.getKind());
+
+ assertEquals(7, comparison.getConflicts().size());
}
}
diff --git a/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/fullcomparison/IdentifierComparisonTest.java b/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/fullcomparison/IdentifierComparisonTest.java
index a8a2c1377..2944b13c3 100644
--- a/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/fullcomparison/IdentifierComparisonTest.java
+++ b/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/fullcomparison/IdentifierComparisonTest.java
@@ -267,13 +267,15 @@ public class IdentifierComparisonTest extends EMFCompareTestBase {
return;
}
- assertEquals(5, conflicts.size());
+ assertEquals(7, conflicts.size());
Conflict periodicalConflict = null;
Conflict titleConflict = null;
+ Conflict titleETypeConflict = null;
Conflict lastNameConflict = null;
Conflict minutesLengthConflict = null;
Conflict readerConflict = null;
+ Conflict readerETypeConflict = null;
for (Conflict conflict : conflicts) {
for (Diff diff : conflict.getDifferences()) {
@@ -294,6 +296,12 @@ public class IdentifierComparisonTest extends EMFCompareTestBase {
} else if (isMatchOf(match, "minutesLength")) {
minutesLengthConflict = conflict;
breakLoop = true;
+ } else if (isMatchOf(match, "reader")) {
+ readerETypeConflict = conflict;
+ breakLoop = true;
+ } else if (isMatchOf(match, "title")) {
+ titleETypeConflict = conflict;
+ breakLoop = true;
}
if (breakLoop) {
break;
@@ -303,16 +311,20 @@ public class IdentifierComparisonTest extends EMFCompareTestBase {
assertNotNull(periodicalConflict);
assertNotNull(titleConflict);
+ assertNotNull(titleETypeConflict);
assertNotNull(lastNameConflict);
assertNotNull(minutesLengthConflict);
assertNotNull(readerConflict);
+ assertNotNull(readerETypeConflict);
// These classic asserts will make the compiler happy.
assert periodicalConflict != null;
assert titleConflict != null;
+ assert titleETypeConflict != null;
assert lastNameConflict != null;
assert minutesLengthConflict != null;
assert readerConflict != null;
+ assert readerETypeConflict != null;
final List<Diff> periodicalDiffs = Lists.newArrayList(periodicalConflict.getDifferences());
assertSame(ConflictKind.REAL, periodicalConflict.getKind());
@@ -341,22 +353,30 @@ public class IdentifierComparisonTest extends EMFCompareTestBase {
final List<Diff> titleDiffs = Lists.newArrayList(titleConflict.getDifferences());
assertSame(ConflictKind.PSEUDO, titleConflict.getKind());
assertRemoved(titleDiffs, "extlibrary.Periodical.title", DifferenceSource.LEFT);
- assertChangedReference(titleDiffs, "extlibrary.Periodical.title", "eType", "ecore.EString", null,
- DifferenceSource.LEFT);
- assertChangedReference(titleDiffs, "extlibrary.Periodical.title", "eType", "ecore.EString", null,
- DifferenceSource.RIGHT);
assertRemoved(titleDiffs, "extlibrary.Periodical.title", DifferenceSource.RIGHT);
assertTrue(titleDiffs.isEmpty());
+ final List<Diff> titleETypeDiffs = Lists.newArrayList(titleETypeConflict.getDifferences());
+ assertSame(ConflictKind.PSEUDO, titleConflict.getKind());
+ assertChangedReference(titleETypeDiffs, "extlibrary.Periodical.title", "eType", "ecore.EString", null,
+ DifferenceSource.LEFT);
+ assertChangedReference(titleETypeDiffs, "extlibrary.Periodical.title", "eType", "ecore.EString", null,
+ DifferenceSource.RIGHT);
+ assertTrue(titleETypeDiffs.isEmpty());
+
final List<Diff> readerDiffs = Lists.newArrayList(readerConflict.getDifferences());
assertSame(ConflictKind.PSEUDO, readerConflict.getKind());
assertRemoved(readerDiffs, "extlibrary.BookOnTape.reader", DifferenceSource.LEFT);
assertRemoved(readerDiffs, "extlibrary.BookOnTape.reader", DifferenceSource.RIGHT);
- assertChangedReference(readerDiffs, "extlibrary.BookOnTape.reader", "eType", "extlibrary.Person",
+ assertTrue(readerDiffs.isEmpty());
+
+ final List<Diff> readerETypeDiffs = Lists.newArrayList(readerETypeConflict.getDifferences());
+ assertSame(ConflictKind.PSEUDO, readerETypeConflict.getKind());
+ assertChangedReference(readerETypeDiffs, "extlibrary.BookOnTape.reader", "eType", "extlibrary.Person",
null, DifferenceSource.LEFT);
- assertChangedReference(readerDiffs, "extlibrary.BookOnTape.reader", "eType", "extlibrary.Person",
+ assertChangedReference(readerETypeDiffs, "extlibrary.BookOnTape.reader", "eType", "extlibrary.Person",
null, DifferenceSource.RIGHT);
- assertTrue(readerDiffs.isEmpty());
+ assertTrue(readerETypeDiffs.isEmpty());
}
private boolean isMatchOf(Match match, String name) {
diff --git a/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/fullcomparison/data/identifier/CHANGES b/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/fullcomparison/data/identifier/CHANGES
index 668142a53..971aabe93 100644
--- a/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/fullcomparison/data/identifier/CHANGES
+++ b/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/fullcomparison/data/identifier/CHANGES
@@ -104,7 +104,9 @@ Expected Differences
Conflicts on :
1 and 15 : Not a generic conflict, relies on ecore-specific business rules : enum literals of the same enum with the same 'value'.
8, 19 and 23 : REAL : locally deleted Periodical. However, we've remotely set "Periodical" as the super-type of Magazine.
- 31, 45, 32 and 46 : PSEUDO : We've removed title both locally and remotely, which is a PSEUDO conflict.
+ 31 and 32 : PSEUDO : We've removed title both locally and remotely, which is a PSEUDO conflict.
+ 45 and 46 : PSEUDO : inferred from the deletion of title from both sides, we've unset the eType of title on both sides. PSEUDO conflict.
12 and 27 : REAL : remote renaming of a locally deleted attribute
14 and 28 : REAL : renaming to different values
- 29, 30, 33 and 34 : PSEUDO : BookOnTape.reader removed from both sides, and thus its type unset on both sides. \ No newline at end of file
+ 29 and 30 : PSEUDO : BookOnTape.reader removed from both sides, and thus its type unset on both sides.
+ 33 and 34 : PSEUDO : inferred from the deletion of reader on both sides, we've unset the eType of reader on both sides. \ No newline at end of file
diff --git a/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/ConflictImplicationsTest_Bug484579.java b/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/ConflictImplicationsTest_Bug484579.java
index c7bc8b645..e131bbf62 100644
--- a/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/ConflictImplicationsTest_Bug484579.java
+++ b/plugins/org.eclipse.emf.compare.tests/src/org/eclipse/emf/compare/tests/merge/ConflictImplicationsTest_Bug484579.java
@@ -27,6 +27,7 @@ import org.eclipse.emf.common.util.EList;
import org.eclipse.emf.compare.Comparison;
import org.eclipse.emf.compare.Conflict;
import org.eclipse.emf.compare.Diff;
+import org.eclipse.emf.compare.DifferenceSource;
import org.eclipse.emf.compare.EMFCompare;
import org.eclipse.emf.compare.merge.IMerger;
import org.eclipse.emf.compare.scope.DefaultComparisonScope;
@@ -43,6 +44,7 @@ import org.junit.Test;
*
* @see bug <a href="https://bugs.eclipse.org/bugs/show_bug.cgi?id=484579">484579</a> for more details.
*/
+@SuppressWarnings("nls")
public class ConflictImplicationsTest_Bug484579 {
private IndividualDiffInputData input = new IndividualDiffInputData();
@@ -148,21 +150,29 @@ public class ConflictImplicationsTest_Bug484579 {
EList<Diff> differences = comparison.getDifferences();
assertEquals(9, comparison.getDifferences().size());
- assertEquals(3, conflicts.size());
+ assertEquals(4, conflicts.size());
Collection<Conflict> pseudoConflicts = Collections2.filter(conflicts,
EMFComparePredicates.containsConflictOfTypes(PSEUDO));
Collection<Conflict> realConflicts = Collections2.filter(conflicts,
EMFComparePredicates.containsConflictOfTypes(REAL));
- assertEquals(1, pseudoConflicts.size());
+ assertEquals(2, pseudoConflicts.size());
assertEquals(2, realConflicts.size());
- EList<Diff> pseudoConflictDifferences = pseudoConflicts.iterator().next().getDifferences();
- Collection<Diff> pseudoConflictsFromLeft = Collections2.filter(pseudoConflictDifferences,
- fromSide(LEFT));
- Collection<Diff> pseudoConflictFromRight = Collections2.filter(pseudoConflictDifferences,
- fromSide(RIGHT));
+ Conflict pseudo1 = null;
+ Conflict pseudo2 = null;
+ for (Conflict conflict : pseudoConflicts) {
+ if (conflict.getLeftDifferences().get(0).getMatch().getOrigin().toString().contains("ClassZ")) {
+ pseudo1 = conflict;
+ } else if (conflict.getLeftDifferences().get(0).getMatch().getOrigin().toString()
+ .contains("PackageY")) {
+ pseudo2 = conflict;
+ }
+ }
+
+ assertNotNull(pseudo1);
+ assertNotNull(pseudo2);
Conflict real1 = null;
Conflict real2 = null;
@@ -199,27 +209,41 @@ public class ConflictImplicationsTest_Bug484579 {
Diff renamePackageA = conflict2FromRight.iterator().next();
differences.remove(renamePackageA);
- // Each pseudo-conflicting diff from left should implies 6 other diff when merging from right to left,
- // and 3 other when merging from left to right.
- for (Diff diff : pseudoConflictsFromLeft) {
+ // Deleting classB from supertypes of classZ.
+ // Rejecting the left side implies 6 others, accepting only implies the pseudo conflicting one
+ // and inversely on the right side diff.
+ for (Diff diff : pseudo1.getDifferences()) {
differences.remove(diff);
checker = getChecker(diff);
- checker.rightToLeft().implies(7).rejects(0).check();
- checker.leftToRight().implies(4).rejects(0).check();
+ if (diff.getSource() == DifferenceSource.LEFT) {
+ checker.rightToLeft().implies(7).rejects(0).check();
+ checker.leftToRight().implies(2).rejects(0).check();
+ } else {
+ checker.rightToLeft().implies(2).rejects(0).check();
+ checker.leftToRight().implies(7).rejects(0).check();
+ }
}
- // Each pseudo-conflicting diff from right should implies 3 other diff when merging from right to
- // left, and 6 other when merging from left to right.
- for (Diff diff : pseudoConflictFromRight) {
+ // Deleting classZ
+ // Rejecting the left side only implies the pseudo-conflicting and rejecting its container's deletion,
+ // while accepting implies both diffs from the other pseudo-conflict. The diff from the right side is
+ // the reverse.
+ // Note that we're not considering cascading diffs.
+ for (Diff diff : pseudo2.getDifferences()) {
differences.remove(diff);
checker = getChecker(diff);
- checker.rightToLeft().implies(4).rejects(0).check();
- checker.leftToRight().implies(7).rejects(0).check();
+ if (diff.getSource() == DifferenceSource.LEFT) {
+ checker.rightToLeft().implies(3).rejects(0).check();
+ checker.leftToRight().implies(4).rejects(0).check();
+ } else {
+ checker.rightToLeft().implies(4).rejects(0).check();
+ checker.leftToRight().implies(3).rejects(0).check();
+ }
}
checker = getChecker(deleteClassB);
checker.rightToLeft().implies(2).rejects(0).check();
- checker.leftToRight().implies(5).rejects(1).check();
+ checker.leftToRight().implies(3).rejects(1).check();
checker = getChecker(renameClassB);
checker.rightToLeft().implies(1).rejects(2).check();
@@ -227,7 +251,7 @@ public class ConflictImplicationsTest_Bug484579 {
checker = getChecker(deletePackageA);
checker.rightToLeft().implies(1).rejects(0).check();
- checker.leftToRight().implies(6).rejects(2).check();
+ checker.leftToRight().implies(4).rejects(2).check();
checker = getChecker(renamePackageA);
checker.rightToLeft().implies(1).rejects(1).check();
@@ -254,14 +278,14 @@ public class ConflictImplicationsTest_Bug484579 {
EList<Conflict> conflicts = comparison.getConflicts();
assertEquals(17, comparison.getDifferences().size());
- assertEquals(3, conflicts.size());
+ assertEquals(7, conflicts.size());
Collection<Conflict> pseudoConflicts = Collections2.filter(conflicts,
EMFComparePredicates.containsConflictOfTypes(PSEUDO));
Collection<Conflict> realConflicts = Collections2.filter(conflicts,
EMFComparePredicates.containsConflictOfTypes(REAL));
- assertEquals(2, pseudoConflicts.size());
+ assertEquals(6, pseudoConflicts.size());
assertEquals(1, realConflicts.size());
return realConflicts;
diff --git a/plugins/org.eclipse.emf.compare/src/org/eclipse/emf/compare/conflict/DefaultConflictDetector.java b/plugins/org.eclipse.emf.compare/src/org/eclipse/emf/compare/conflict/DefaultConflictDetector.java
index 8cd339d9e..5ebd735fa 100644
--- a/plugins/org.eclipse.emf.compare/src/org/eclipse/emf/compare/conflict/DefaultConflictDetector.java
+++ b/plugins/org.eclipse.emf.compare/src/org/eclipse/emf/compare/conflict/DefaultConflictDetector.java
@@ -247,7 +247,10 @@ public class DefaultConflictDetector implements IConflictDetector {
while (diffIterator.hasNext()) {
Diff extendedCandidate = diffIterator.next();
if (isDeleteOrUnsetDiff(extendedCandidate)) {
- conflictOn(comparison, diff, extendedCandidate, ConflictKind.PSEUDO);
+ // We do not want to create a pseudo conflict between a deleted container and its deleted
+ // content, since that would prevent us from merging the container deletion altogether
+ // (since pseudo conflicts usually mean that no action is needed).
+ // conflictOn(comparison, diff, extendedCandidate, ConflictKind.PSEUDO);
} else {
conflictOn(comparison, diff, extendedCandidate, ConflictKind.REAL);
}
@@ -397,7 +400,10 @@ public class DefaultConflictDetector implements IConflictDetector {
while (diffIterator.hasNext()) {
Diff extendedCandidate = diffIterator.next();
if (isDeleteOrUnsetDiff(extendedCandidate)) {
- conflictOn(comparison, diff, extendedCandidate, ConflictKind.PSEUDO);
+ // We do not want to create a pseudo conflict between a deleted container and its deleted
+ // content, since that would prevent us from merging the container deletion altogether
+ // (since pseudo conflicts usually mean that no action is needed).
+ // conflictOn(comparison, diff, extendedCandidate, ConflictKind.PSEUDO);
} else {
conflictOn(comparison, diff, extendedCandidate, ConflictKind.REAL);
}
@@ -999,7 +1005,10 @@ public class DefaultConflictDetector implements IConflictDetector {
for (Diff extendedCandidate : Iterables.filter(match.getAllDifferences(),
possiblyConflictingWith(diff))) {
if (isDeleteOrUnsetDiff(extendedCandidate)) {
- conflictOn(comparison, diff, extendedCandidate, ConflictKind.PSEUDO);
+ // We do not want to create a pseudo conflict between a deleted container and its
+ // deleted content, since that would prevent us from merging the container deletion
+ // altogether (since pseudo conflicts usually mean that no action is needed).
+ // conflictOn(comparison, diff, extendedCandidate, ConflictKind.PSEUDO);
} else {
conflictOn(comparison, diff, extendedCandidate, ConflictKind.REAL);
}
diff --git a/plugins/org.eclipse.emf.compare/src/org/eclipse/emf/compare/internal/conflict/ContainmentRefChangeConflictSearch.java b/plugins/org.eclipse.emf.compare/src/org/eclipse/emf/compare/internal/conflict/ContainmentRefChangeConflictSearch.java
index 483f15bc8..ddaa96e2b 100644
--- a/plugins/org.eclipse.emf.compare/src/org/eclipse/emf/compare/internal/conflict/ContainmentRefChangeConflictSearch.java
+++ b/plugins/org.eclipse.emf.compare/src/org/eclipse/emf/compare/internal/conflict/ContainmentRefChangeConflictSearch.java
@@ -225,7 +225,10 @@ public class ContainmentRefChangeConflictSearch {
while (diffIterator.hasNext()) {
Diff extendedCandidate = diffIterator.next();
if (isDeleteOrUnsetDiff(extendedCandidate)) {
- conflict(extendedCandidate, PSEUDO);
+ // We do not want to create a pseudo conflict between a deleted container and its
+ // deleted content, since that would prevent us from merging the container deletion
+ // altogether (since pseudo conflicts usually mean that no action is needed).
+ // conflict(extendedCandidate, PSEUDO);
} else {
conflict(extendedCandidate, REAL);
}
diff --git a/plugins/org.eclipse.emf.compare/src/org/eclipse/emf/compare/internal/conflict/ResourceAttachmentChangeConflictSearch.java b/plugins/org.eclipse.emf.compare/src/org/eclipse/emf/compare/internal/conflict/ResourceAttachmentChangeConflictSearch.java
index e40f0eac6..13eca9cc1 100644
--- a/plugins/org.eclipse.emf.compare/src/org/eclipse/emf/compare/internal/conflict/ResourceAttachmentChangeConflictSearch.java
+++ b/plugins/org.eclipse.emf.compare/src/org/eclipse/emf/compare/internal/conflict/ResourceAttachmentChangeConflictSearch.java
@@ -220,7 +220,10 @@ public class ResourceAttachmentChangeConflictSearch {
for (Diff extendedCandidate : Iterables.filter(match.getDifferences(),
possiblyConflictingWith(diff))) {
if (isDeleteOrUnsetDiff(extendedCandidate)) {
- conflict(extendedCandidate, PSEUDO);
+ // We do not want to create a pseudo conflict between a deleted container and its
+ // deleted content, since that would prevent us from merging the container deletion
+ // altogether (since pseudo conflicts usually mean that no action is needed).
+ // conflict(extendedCandidate, PSEUDO);
} else {
conflict(extendedCandidate, REAL);
}
diff --git a/plugins/org.eclipse.emf.compare/src/org/eclipse/emf/compare/merge/AttributeChangeMerger.java b/plugins/org.eclipse.emf.compare/src/org/eclipse/emf/compare/merge/AttributeChangeMerger.java
index 567525ee0..9bf5a2c98 100644
--- a/plugins/org.eclipse.emf.compare/src/org/eclipse/emf/compare/merge/AttributeChangeMerger.java
+++ b/plugins/org.eclipse.emf.compare/src/org/eclipse/emf/compare/merge/AttributeChangeMerger.java
@@ -316,9 +316,15 @@ public class AttributeChangeMerger extends AbstractMerger {
* Direction of the merge.
*/
protected void changeValue(AttributeChange diff, boolean rightToLeft) {
+ final EObject targetContainer = getTargetContainer(diff, rightToLeft);
+ if (targetContainer == null) {
+ // We're merging the unset of an attribute value under a containment deletion.
+ // There's actually no action to take in such a case.
+ return;
+ }
+
final EStructuralFeature attribute = diff.getAttribute();
final Object targetValue = getTargetValue(diff, rightToLeft);
- final EObject targetContainer = getTargetContainer(diff, rightToLeft);
if (isUnset(diff, targetValue)) {
targetContainer.eUnset(attribute);

Back to the top