From 95717734f37961a87069c3736bf977adc91bffaa Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Fri, 6 Sep 2024 14:55:47 +0200 Subject: [PATCH] Add tests --- .../system/OutsideSearchWindowFilter.java | 2 +- .../model/plan/TestItineraryBuilder.java | 4 ++ .../system/OutsideSearchWindowFilterTest.java | 47 +++++++++++++++---- 3 files changed, 44 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/opentripplanner/routing/algorithm/filterchain/filters/system/OutsideSearchWindowFilter.java b/src/main/java/org/opentripplanner/routing/algorithm/filterchain/filters/system/OutsideSearchWindowFilter.java index 5192972102c..407a2f5f5b8 100644 --- a/src/main/java/org/opentripplanner/routing/algorithm/filterchain/filters/system/OutsideSearchWindowFilter.java +++ b/src/main/java/org/opentripplanner/routing/algorithm/filterchain/filters/system/OutsideSearchWindowFilter.java @@ -52,7 +52,7 @@ public Predicate shouldBeFlaggedForRemoval() { // end up time-shifted right up to the arrive by time. // further reading: https://github.com/opentripplanner/OpenTripPlanner/issues/6046 if (it.isOnStreetAndFlexOnly() && searchDirection.isArriveBy()) { - return startTime.isBefore(latestDepartureTime); + return startTime.isBefore(earliestDepartureTime); } else { return ( startTime.isBefore(earliestDepartureTime) || !startTime.isBefore(latestDepartureTime) diff --git a/src/test/java/org/opentripplanner/model/plan/TestItineraryBuilder.java b/src/test/java/org/opentripplanner/model/plan/TestItineraryBuilder.java index edaafabd753..3fa75aa99fe 100644 --- a/src/test/java/org/opentripplanner/model/plan/TestItineraryBuilder.java +++ b/src/test/java/org/opentripplanner/model/plan/TestItineraryBuilder.java @@ -199,9 +199,13 @@ public TestItineraryBuilder flex(int start, int end, Place to) { int legCost = 0; StopTime fromStopTime = new StopTime(); fromStopTime.setStop(lastPlace.stop); + fromStopTime.setFlexWindowStart(start); + fromStopTime.setFlexWindowStart(end); StopTime toStopTime = new StopTime(); toStopTime.setStop(to.stop); + toStopTime.setFlexWindowStart(start); + toStopTime.setFlexWindowStart(end); Trip trip = trip("1", route("flex").build()); diff --git a/src/test/java/org/opentripplanner/routing/algorithm/filterchain/filters/system/OutsideSearchWindowFilterTest.java b/src/test/java/org/opentripplanner/routing/algorithm/filterchain/filters/system/OutsideSearchWindowFilterTest.java index bd580ae4934..18d08028022 100644 --- a/src/test/java/org/opentripplanner/routing/algorithm/filterchain/filters/system/OutsideSearchWindowFilterTest.java +++ b/src/test/java/org/opentripplanner/routing/algorithm/filterchain/filters/system/OutsideSearchWindowFilterTest.java @@ -1,17 +1,19 @@ package org.opentripplanner.routing.algorithm.filterchain.filters.system; +import static com.google.common.truth.Truth.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.opentripplanner.framework.time.TimeUtils.time; import static org.opentripplanner.model.plan.TestItineraryBuilder.newItinerary; import java.time.Duration; import java.util.List; +import java.util.stream.Stream; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; -import org.opentripplanner.framework.time.TimeUtils; import org.opentripplanner.model.SystemNotice; import org.opentripplanner.model.plan.Itinerary; import org.opentripplanner.model.plan.PlanTestConstants; @@ -21,11 +23,13 @@ class OutsideSearchWindowFilterTest implements PlanTestConstants { private static final Duration SEARCH_WINDOW_10m = Duration.ofMinutes(10); - private final int startTime = TimeUtils.time("09:30"); - private final int endTime = TimeUtils.time("09:40"); + private static final int START_TIME = time("09:30"); + private static final int END_TIME = time("09:40"); - private final Itinerary itinerary = newItinerary(A).bus(32, startTime, endTime, E).build(); - private final List input = List.of(itinerary); + private static final Itinerary BUS_ITINERARY = newItinerary(A) + .bus(32, START_TIME, END_TIME, E) + .build(); + private static final List INPUT = List.of(BUS_ITINERARY); static List filterOnSearchWindowTestCases() { return List.of( @@ -39,13 +43,13 @@ static List filterOnSearchWindowTestCases() { @ParameterizedTest(name = "{0}, edt: {1}, sw: 10m, expects flagged for removal: {2}") @MethodSource("filterOnSearchWindowTestCases") void filterOnSearchWindow(String description, String edt, boolean flaggedForRemoval) { - List expected = flaggedForRemoval ? input : List.of(); + List expected = flaggedForRemoval ? INPUT : List.of(); var subject = new OutsideSearchWindowFilter( - TestItineraryBuilder.newTime(TimeUtils.time(edt)).toInstant(), + TestItineraryBuilder.newTime(time(edt)).toInstant(), SEARCH_WINDOW_10m, SearchDirection.DEPART_AT ); - var result = subject.flagForRemoval(input); + var result = subject.flagForRemoval(INPUT); assertEquals(expected, result, description); } @@ -57,4 +61,31 @@ public void testTaggedBy() { it.flagForDeletion(new SystemNotice(OutsideSearchWindowFilter.TAG, "Text")); assertTrue(OutsideSearchWindowFilter.taggedBy(it)); } + + private static Stream onStreetTestCases() { + int t9_28 = time("9:28"); + int t9_38 = time("9:38"); + return Stream + .of( + newItinerary(A, t9_28).walk(D2m, B), + newItinerary(A, t9_38).walk(D12m, B), + newItinerary(A, t9_28).bicycle(t9_28, t9_38, B), + newItinerary(A, t9_28).flex(t9_28, t9_38, B), + newItinerary(A, t9_28).flex(t9_38, time("9:48"), B), + newItinerary(A, time("9:20")).flex(time("9:20"), t9_28, B).walk(D12m, C) + ) + .map(TestItineraryBuilder::build); + } + + @ParameterizedTest + @MethodSource("onStreetTestCases") + void onStreetArriveByShouldNotBeRemoved(Itinerary itin) { + var edt = "9:20"; + var subject = new OutsideSearchWindowFilter( + TestItineraryBuilder.newTime(time(edt)).toInstant(), + SEARCH_WINDOW_10m, + SearchDirection.ARRIVE_BY + ); + assertThat(subject.flagForRemoval(List.of(itin))).isEmpty(); + } }