From d27d63013508f8d8e1e401926a2856311a1254b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20Locker?= Date: Mon, 6 Jun 2016 10:28:52 +0200 Subject: [PATCH] Add code coverage for phase lifecycle and fix a few bugs (#194) * Add phase lifecycle test & fix wrong solvingStarted() calls * Solving started/ended is now called only once per solve() execution * Fix DefaultSolver#removePhaseLifecycleListener(listener) --- .../DefaultConstructionHeuristicPhase.java | 2 +- .../DefaultExhaustiveSearchPhase.java | 2 +- .../core/impl/phase/AbstractPhase.java | 2 - .../core/impl/solver/DefaultSolver.java | 8 +- .../core/impl/phase/PhaseLifecyleTest.java | 85 +++++++++++++++++++ .../impl/testdata/util/PlannerTestUtils.java | 4 +- 6 files changed, 97 insertions(+), 6 deletions(-) create mode 100644 optaplanner-core/src/test/java/org/optaplanner/core/impl/phase/PhaseLifecyleTest.java diff --git a/optaplanner-core/src/main/java/org/optaplanner/core/impl/constructionheuristic/DefaultConstructionHeuristicPhase.java b/optaplanner-core/src/main/java/org/optaplanner/core/impl/constructionheuristic/DefaultConstructionHeuristicPhase.java index c1bcde7695..c2dfa0be55 100644 --- a/optaplanner-core/src/main/java/org/optaplanner/core/impl/constructionheuristic/DefaultConstructionHeuristicPhase.java +++ b/optaplanner-core/src/main/java/org/optaplanner/core/impl/constructionheuristic/DefaultConstructionHeuristicPhase.java @@ -148,7 +148,7 @@ public void phaseEnded(ConstructionHeuristicPhaseScope phaseScope) { @Override public void solvingEnded(DefaultSolverScope solverScope) { - super.solvingStarted(solverScope); + super.solvingEnded(solverScope); entityPlacer.solvingEnded(solverScope); decider.solvingEnded(solverScope); } diff --git a/optaplanner-core/src/main/java/org/optaplanner/core/impl/exhaustivesearch/DefaultExhaustiveSearchPhase.java b/optaplanner-core/src/main/java/org/optaplanner/core/impl/exhaustivesearch/DefaultExhaustiveSearchPhase.java index a78fa2fc9e..0c383c2917 100644 --- a/optaplanner-core/src/main/java/org/optaplanner/core/impl/exhaustivesearch/DefaultExhaustiveSearchPhase.java +++ b/optaplanner-core/src/main/java/org/optaplanner/core/impl/exhaustivesearch/DefaultExhaustiveSearchPhase.java @@ -259,7 +259,7 @@ public void phaseEnded(ExhaustiveSearchPhaseScope phaseScope) { @Override public void solvingEnded(DefaultSolverScope solverScope) { - super.solvingStarted(solverScope); + super.solvingEnded(solverScope); entitySelector.solvingEnded(solverScope); decider.solvingEnded(solverScope); } diff --git a/optaplanner-core/src/main/java/org/optaplanner/core/impl/phase/AbstractPhase.java b/optaplanner-core/src/main/java/org/optaplanner/core/impl/phase/AbstractPhase.java index fc1b8db661..697afd58ed 100644 --- a/optaplanner-core/src/main/java/org/optaplanner/core/impl/phase/AbstractPhase.java +++ b/optaplanner-core/src/main/java/org/optaplanner/core/impl/phase/AbstractPhase.java @@ -107,14 +107,12 @@ public void setAssertShadowVariablesAreNotStaleAfterStep(boolean assertShadowVar public void solvingStarted(DefaultSolverScope solverScope) { // bestSolutionRecaller.solvingStarted(...) is called by DefaultSolver termination.solvingStarted(solverScope); - phaseLifecycleSupport.fireSolvingStarted(solverScope); } @Override public void solvingEnded(DefaultSolverScope solverScope) { // bestSolutionRecaller.solvingEnded(...) is called by DefaultSolver termination.solvingEnded(solverScope); - phaseLifecycleSupport.fireSolvingEnded(solverScope); } @Override diff --git a/optaplanner-core/src/main/java/org/optaplanner/core/impl/solver/DefaultSolver.java b/optaplanner-core/src/main/java/org/optaplanner/core/impl/solver/DefaultSolver.java index 2c48bb00b4..48ed4131fd 100644 --- a/optaplanner-core/src/main/java/org/optaplanner/core/impl/solver/DefaultSolver.java +++ b/optaplanner-core/src/main/java/org/optaplanner/core/impl/solver/DefaultSolver.java @@ -28,6 +28,7 @@ import org.optaplanner.core.config.solver.EnvironmentMode; import org.optaplanner.core.impl.phase.Phase; import org.optaplanner.core.impl.phase.event.PhaseLifecycleListener; +import org.optaplanner.core.impl.phase.event.PhaseLifecycleSupport; import org.optaplanner.core.impl.score.director.InnerScoreDirectorFactory; import org.optaplanner.core.impl.solver.event.SolverEventSupport; import org.optaplanner.core.impl.solver.random.RandomFactory; @@ -47,6 +48,7 @@ public class DefaultSolver implements Solver { protected final transient Logger logger = LoggerFactory.getLogger(getClass()); protected SolverEventSupport solverEventSupport = new SolverEventSupport<>(this); + protected PhaseLifecycleSupport phaseLifecycleSupport = new PhaseLifecycleSupport<>(); protected EnvironmentMode environmentMode; protected RandomFactory randomFactory; @@ -214,6 +216,7 @@ public void solvingStarted(DefaultSolverScope solverScope) { } int startingSolverCount = solverScope.getStartingSolverCount() + 1; solverScope.setStartingSolverCount(startingSolverCount); + phaseLifecycleSupport.fireSolvingStarted(solverScope); logger.info("Solving {}: time spent ({}), best score ({}), environment mode ({}), random ({}).", (startingSolverCount == 1 ? "started" : "restarted"), solverScope.calculateTimeMillisSpentUpToNow(), @@ -240,6 +243,7 @@ public void solvingEnded(DefaultSolverScope solverScope) { } bestSolutionRecaller.solvingEnded(solverScope); solverScope.endingNow(); + phaseLifecycleSupport.fireSolvingEnded(solverScope); } public void outerSolvingEnded(DefaultSolverScope solverScope) { @@ -297,14 +301,16 @@ public void removeEventListener(SolverEventListener eventListener) { } public void addPhaseLifecycleListener(PhaseLifecycleListener phaseLifecycleListener) { + phaseLifecycleSupport.addEventListener(phaseLifecycleListener); for (Phase phase : phaseList) { phase.addPhaseLifecycleListener(phaseLifecycleListener); } } public void removePhaseLifecycleListener(PhaseLifecycleListener phaseLifecycleListener) { + phaseLifecycleSupport.removeEventListener(phaseLifecycleListener); for (Phase phase : phaseList) { - phase.addPhaseLifecycleListener(phaseLifecycleListener); + phase.removePhaseLifecycleListener(phaseLifecycleListener); } } diff --git a/optaplanner-core/src/test/java/org/optaplanner/core/impl/phase/PhaseLifecyleTest.java b/optaplanner-core/src/test/java/org/optaplanner/core/impl/phase/PhaseLifecyleTest.java new file mode 100644 index 0000000000..942dcf0920 --- /dev/null +++ b/optaplanner-core/src/test/java/org/optaplanner/core/impl/phase/PhaseLifecyleTest.java @@ -0,0 +1,85 @@ +/* + * Copyright 2016 Red Hat, Inc. and/or its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.optaplanner.core.impl.phase; + +import java.util.ArrayList; +import java.util.Arrays; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.runners.MockitoJUnitRunner; +import org.optaplanner.core.api.solver.Solver; +import org.optaplanner.core.api.solver.SolverFactory; +import org.optaplanner.core.impl.phase.event.PhaseLifecycleListener; +import org.optaplanner.core.impl.solver.DefaultSolver; +import org.optaplanner.core.impl.testdata.domain.TestdataEntity; +import org.optaplanner.core.impl.testdata.domain.TestdataSolution; +import org.optaplanner.core.impl.testdata.domain.TestdataValue; +import org.optaplanner.core.impl.testdata.util.PlannerAssert; +import org.optaplanner.core.impl.testdata.util.PlannerTestUtils; + +@RunWith(MockitoJUnitRunner.class) +public class PhaseLifecyleTest { + + @Mock + private PhaseLifecycleListener listener; + + @Test + public void verifyEventCounts() { + // prepare solver + SolverFactory solverFactory = PlannerTestUtils.buildSolverFactory( + TestdataSolution.class, TestdataEntity.class); + Solver solver = solverFactory.buildSolver(); + + // prepare solution + TestdataSolution solution = new TestdataSolution("s1"); + solution.setValueList(Arrays.asList(new TestdataValue("v1"), new TestdataValue("v1"))); + final int entitiesCount = 17; + ArrayList entities = new ArrayList<>(entitiesCount); + for (int i = 0; i < entitiesCount; i++) { + entities.add(new TestdataEntity(String.valueOf(i))); + } + solution.setEntityList(entities); + + // add listener mock and solve + ((DefaultSolver) solver).addPhaseLifecycleListener(listener); + TestdataSolution solvedSolution = solver.solve(solution); + + // step count = number of uninitialized entities (CH) + LS step count limit + final int stepCount = entitiesCount + PlannerTestUtils.TERMINATION_STEP_COUNT_LIMIT; + final int phaseCount = solverFactory.getSolverConfig().getPhaseConfigList().size(); + final int solvingCount = 1; + PlannerAssert.verifyPhaseLifecycle(listener, solvingCount, phaseCount, stepCount); + + // forget previous invocations + Mockito.>reset(listener); + + // uninitialize 1 entity and solve again + solvedSolution.getEntityList().get(0).setValue(null); + solver.solve(solvedSolution); + PlannerAssert.verifyPhaseLifecycle(listener, solvingCount, phaseCount, 1 + PlannerTestUtils.TERMINATION_STEP_COUNT_LIMIT); + + // forget previous invocations + Mockito.>reset(listener); + + // remove listener and solve again + ((DefaultSolver) solver).removePhaseLifecycleListener(listener); + solver.solve(solution); + PlannerAssert.verifyPhaseLifecycle(listener, 0, 0, 0); + } +} diff --git a/optaplanner-core/src/test/java/org/optaplanner/core/impl/testdata/util/PlannerTestUtils.java b/optaplanner-core/src/test/java/org/optaplanner/core/impl/testdata/util/PlannerTestUtils.java index 3b9314d438..1ad174b323 100644 --- a/optaplanner-core/src/test/java/org/optaplanner/core/impl/testdata/util/PlannerTestUtils.java +++ b/optaplanner-core/src/test/java/org/optaplanner/core/impl/testdata/util/PlannerTestUtils.java @@ -52,6 +52,8 @@ */ public class PlannerTestUtils { + public static final int TERMINATION_STEP_COUNT_LIMIT = 10; + // ************************************************************************ // SolverFactory methods // ************************************************************************ @@ -70,7 +72,7 @@ public static SolverFactory buildSolverFactory( phaseConfigList.add(new ConstructionHeuristicPhaseConfig()); LocalSearchPhaseConfig localSearchPhaseConfig = new LocalSearchPhaseConfig(); TerminationConfig terminationConfig = new TerminationConfig(); - terminationConfig.setStepCountLimit(10); + terminationConfig.setStepCountLimit(TERMINATION_STEP_COUNT_LIMIT); localSearchPhaseConfig.setTerminationConfig(terminationConfig); phaseConfigList.add(localSearchPhaseConfig); solverConfig.setPhaseConfigList(phaseConfigList);