Fix the final test

Author: triceo

core/src/main/java/ai/timefold/solver/core/impl/move/streams/maybeapi/generic/provider/ListChangeMoveProvider.java

@@ -1,7 +1,5 @@
 package ai.timefold.solver.core.impl.move.streams.maybeapi.generic.provider;
 
-import java.util.Objects;
-
 import ai.timefold.solver.core.impl.move.streams.maybeapi.BiDataFilter;
 import ai.timefold.solver.core.impl.move.streams.maybeapi.DataJoiners;
 import ai.timefold.solver.core.impl.move.streams.maybeapi.generic.move.ListAssignMove;
@@ -14,9 +12,10 @@
 import ai.timefold.solver.core.preview.api.domain.metamodel.PlanningListVariableMetaModel;
 import ai.timefold.solver.core.preview.api.domain.metamodel.PositionInList;
 import ai.timefold.solver.core.preview.api.domain.metamodel.UnassignedElement;
-
 import org.jspecify.annotations.NullMarked;
 
+import java.util.Objects;
+
 @NullMarked
 public class ListChangeMoveProvider<Solution_, Entity_, Value_>
         implements MoveProvider<Solution_> {
@@ -38,10 +37,11 @@ public ListChangeMoveProvider(PlanningListVariableMetaModel<Solution_, Entity_,
         };
         this.validChangeFilter = (solutionView, value, targetPosition) -> {
             var currentPosition = solutionView.getPositionOf(variableMetaModel, value);
-            if (currentPosition.equals(targetPosition)) {
+            if (currentPosition.equals(targetPosition)) { // No change needed.
                 return false;
             }
             if (currentPosition instanceof UnassignedElement) {
+                // Only assign the value if the target entity will accept it.
                 var targetPositionInList = targetPosition.ensureAssigned();
                 return solutionView.isValueInRange(variableMetaModel, targetPositionInList.entity(), value);
             } else {
@@ -53,11 +53,13 @@ public ListChangeMoveProvider(PlanningListVariableMetaModel<Solution_, Entity_,
                     var valueCount = solutionView.countValues(variableMetaModel, currentPositionInList.entity());
                     if (valueCount == 1) {
                         return false; // The value is already in the list, and it is the only one.
-                    } else {
+                    } else if (targetPositionInList.index() == valueCount) {
+                        // The value is already in the list, and we are trying to move it past the end of the list.
+                        return false;
+                    } else { // Same list, same position; ignore.
                         return currentPositionInList.index() != targetPositionInList.index();
                     }
-                } else {
-                    // We can move freely between entities.
+                } else { // We can move freely between entities, assuming the target entity accepts the value.
                     return solutionView.isValueInRange(variableMetaModel, targetPositionInList.entity(), value);
                 }
             }
@@ -66,7 +68,7 @@ public ListChangeMoveProvider(PlanningListVariableMetaModel<Solution_, Entity_,
 
     @Override
     public MoveProducer<Solution_> apply(MoveStreamFactory<Solution_> moveStreamFactory) {
-        // For each unassigned value, we need to create a move to assign it to same position of some list variable.
+        // For each unassigned value, we need to create a move to assign it to some position of some list variable.
         // For each assigned value that is not pinned, we need to create:
         // - A move to unassign it.
         // - A move to reassign it to another position if assigned.

core/src/test/java/ai/timefold/solver/core/impl/move/streams/maybeapi/provider/ChangeMoveProviderTest.java

@@ -1,5 +1,11 @@
 package ai.timefold.solver.core.impl.move.streams.maybeapi.provider;
 
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.SoftAssertions.assertSoftly;
+
+import java.util.Collections;
+import java.util.stream.StreamSupport;
+
 import ai.timefold.solver.core.api.score.buildin.simple.SimpleScore;
 import ai.timefold.solver.core.api.score.stream.Constraint;
 import ai.timefold.solver.core.api.score.stream.ConstraintFactory;
@@ -24,15 +30,10 @@
 import ai.timefold.solver.core.testdomain.valuerange.entityproviding.unassignedvar.TestdataAllowsUnassignedEntityProvidingSolution;
 import ai.timefold.solver.core.testdomain.valuerange.incomplete.TestdataIncompleteValueRangeEntity;
 import ai.timefold.solver.core.testdomain.valuerange.incomplete.TestdataIncompleteValueRangeSolution;
+
 import org.jspecify.annotations.NullMarked;
 import org.junit.jupiter.api.Test;
 
-import java.util.Collections;
-import java.util.stream.StreamSupport;
-
-import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.SoftAssertions.assertSoftly;
-
 @NullMarked
 class ChangeMoveProviderTest {
 

core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java

@@ -202,7 +202,7 @@ void solveWithMoveStreams() {
     }
 
     @Test
-    void solveListVarWithMoveStreams() {
+    void solveWithMoveStreamsListVar() {
         var solverConfig = new SolverConfig()
                 .withPreviewFeature(PreviewFeature.MOVE_STREAMS)
                 .withSolutionClass(TestdataListSolution.class)
@@ -213,9 +213,16 @@ void solveListVarWithMoveStreams() {
                 .withPhases(new LocalSearchPhaseConfig()
                         .withMoveProvidersClass(TestingListMoveProviders.class));
 
-        var solution = TestdataListSolution.generateInitializedSolution(3, 2);
+        // Both values are on the same entity; the goal of the solver is to move one of them to the other entity.
+        var solution = TestdataListSolution.generateUninitializedSolution(2, 2);
+        var v1 = solution.getValueList().get(0);
+        var v2 = solution.getValueList().get(1);
+        var e1 = solution.getEntityList().get(0);
+        e1.addValue(v1);
+        e1.addValue(v2);
+        solution.getEntityList().forEach(TestdataListEntity::setUpShadowVariables);
 
-        solution = PlannerTestUtils.solve(solverConfig, solution, false);
+        solution = PlannerTestUtils.solve(solverConfig, solution, true);
         assertThat(solution).isNotNull();
     }
 
@@ -2389,12 +2396,14 @@ public void afterEntityRemoved(@NonNull Object entity) {
 
     @NullMarked
     public static final class TestingMoveProviders implements MoveProviders<TestdataSolution> {
+
         @Override
         public List<MoveProvider<TestdataSolution>> defineMoves(PlanningSolutionMetaModel<TestdataSolution> solutionMetaModel) {
             var variableMetamodel = solutionMetaModel.entity(TestdataEntity.class)
-                    .<TestdataValue> planningVariable("value");
+                    .<TestdataValue> planningVariable();
             return List.of(new ChangeMoveProvider<>(variableMetamodel));
         }
+
     }
 
     /**
@@ -2419,13 +2428,15 @@ public static final class TestingEasyScoreCalculator implements EasyScoreCalcula
 
     @NullMarked
     public static final class TestingListMoveProviders implements MoveProviders<TestdataListSolution> {
+
         @Override
         public List<MoveProvider<TestdataListSolution>>
                 defineMoves(PlanningSolutionMetaModel<TestdataListSolution> solutionMetaModel) {
             var variableMetamodel = solutionMetaModel.entity(TestdataListEntity.class)
-                    .planningListVariable("valueList");
+                    .planningListVariable();
             return List.of(new ListChangeMoveProvider<>(variableMetamodel));
         }
+
     }
 
     /**

core/src/test/java/ai/timefold/solver/core/testdomain/list/TestdataListEntity.java

@@ -73,4 +73,5 @@ public void removeValue(TestdataListValue value) {
                 .filter(v -> !Objects.equals(v, value))
                 .toList();
     }
+
 }