Skip to content

Commit 47aadd7

Browse files
committed
remove arriveBy matching departAt test because we no longer guarantees it.
1 parent c38737d commit 47aadd7

6 files changed

Lines changed: 19 additions & 40 deletions

File tree

application/src/main/java/org/opentripplanner/street/search/strategy/DominanceFunctions.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,13 @@ public boolean betterOrEqualAndComparable(State a, State b) {
7676
*
7777
* More discussion: https://github.com/opentripplanner/OpenTripPlanner/issues/3393
7878
*
79+
* In addition, the best result may not always be found because, due to the turn cost depending on
80+
* where you come from, we may not always visit the node from the edge producing the optimal cost
81+
* including turns. As a result of this non-optimality, we no longer guarantees that a depart-from
82+
* and an arrive-by search will always return the same result.
83+
*
84+
* Issue: https://github.com/opentripplanner/OpenTripPlanner/issues/6768
85+
*
7986
* == Bicycles ==
8087
*
8188
* We used to allow also loops for bicycles as turn restrictions also apply to them, however

application/src/test/java/org/opentripplanner/routing/algorithm/mapping/BikeRentalSnapshotTest.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ public void directBikeRental() {
6767
.withTo(p2)
6868
.buildRequest();
6969

70-
expectArriveByToMatchDepartAtAndSnapshot(request);
70+
expectRequestResponseToMatchSnapshot(request);
7171
}
7272

7373
/**
@@ -124,7 +124,7 @@ public void accessBikeRental() {
124124
.buildRequest();
125125

126126
try {
127-
expectArriveByToMatchDepartAtAndSnapshot(request);
127+
expectRequestResponseToMatchSnapshot(request);
128128
} catch (CompletionException e) {
129129
RoutingValidationException.unwrapAndRethrowCompletionException(e);
130130
}
@@ -148,7 +148,7 @@ public void egressBikeRental() {
148148
.withTo(p1)
149149
.buildRequest();
150150

151-
expectArriveByToMatchDepartAtAndSnapshot(request);
151+
expectRequestResponseToMatchSnapshot(request);
152152
}
153153

154154
private void allowArrivalWithRentalVehicle(RouteRequestBuilder builder) {

application/src/test/java/org/opentripplanner/routing/algorithm/mapping/CarSnapshotTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ public void directCarPark() {
7171
.withTo(p2)
7272
.buildRequest();
7373

74-
expectArriveByToMatchDepartAtAndSnapshot(request);
74+
expectRequestResponseToMatchSnapshot(request);
7575
}
7676

7777
@DisplayName("Direct CAR_PICKUP (with walking both ends)")
@@ -118,6 +118,6 @@ public void directCarPickupWithoutWalking() {
118118
.withPreferences(pref -> pref.withWalk(w -> w.withSpeed(1.0)))
119119
.buildRequest();
120120

121-
expectArriveByToMatchDepartAtAndSnapshot(request);
121+
expectRequestResponseToMatchSnapshot(request);
122122
}
123123
}

application/src/test/java/org/opentripplanner/routing/algorithm/mapping/ElevationSnapshotTest.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ public void directBikeRental() {
8484
.withTo(p2)
8585
.buildRequest();
8686

87-
expectArriveByToMatchDepartAtAndSnapshot(request);
87+
expectRequestResponseToMatchSnapshot(request);
8888
}
8989

9090
@DisplayName("Direct BIKE")
@@ -124,7 +124,7 @@ public void accessBikeRental() {
124124
.buildRequest();
125125

126126
try {
127-
expectArriveByToMatchDepartAtAndSnapshot(request);
127+
expectRequestResponseToMatchSnapshot(request);
128128
} catch (CompletionException e) {
129129
RoutingValidationException.unwrapAndRethrowCompletionException(e);
130130
}
@@ -139,7 +139,7 @@ public void transit() {
139139
.withTo(p1)
140140
.buildRequest();
141141

142-
expectArriveByToMatchDepartAtAndSnapshot(request);
142+
expectRequestResponseToMatchSnapshot(request);
143143
}
144144

145145
@Override

application/src/test/java/org/opentripplanner/routing/algorithm/mapping/SnapshotTestBase.java

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -181,34 +181,6 @@ protected void expectRequestResponseToMatchSnapshot(RouteRequest request) {
181181
logDebugInformationOnFailure(request, () -> expectItinerariesToMatchSnapshot(itineraries));
182182
}
183183

184-
protected void expectArriveByToMatchDepartAtAndSnapshot(RouteRequest request) {
185-
List<Itinerary> departByItineraries = retrieveItineraries(request);
186-
187-
logDebugInformationOnFailure(request, () -> assertFalse(departByItineraries.isEmpty()));
188-
189-
logDebugInformationOnFailure(request, () ->
190-
expectItinerariesToMatchSnapshot(departByItineraries)
191-
);
192-
193-
RouteRequest arriveBy = request
194-
.copyOf()
195-
.withArriveBy(true)
196-
.withDateTime(departByItineraries.get(0).legs().getLast().endTime().toInstant())
197-
.buildRequest();
198-
199-
List<Itinerary> arriveByItineraries = retrieveItineraries(arriveBy);
200-
201-
var departAtItinerary = departByItineraries.get(0);
202-
var arriveByItinerary = arriveByItineraries.get(0);
203-
204-
logDebugInformationOnFailure(arriveBy, () ->
205-
assertEquals(
206-
asJsonString(itineraryMapper.mapItinerary(departAtItinerary)),
207-
asJsonString(itineraryMapper.mapItinerary(arriveByItinerary))
208-
)
209-
);
210-
}
211-
212184
protected void expectItinerariesToMatchSnapshot(List<Itinerary> itineraries) {
213185
expect(itineraryMapper.mapItineraries(itineraries))
214186
.serializer(snapshotSerializer)

application/src/test/java/org/opentripplanner/routing/algorithm/mapping/TransitSnapshotTest.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ public void test_trip_planning_with_walk_only() {
6868
.withTo(p2)
6969
.buildRequest();
7070

71-
expectArriveByToMatchDepartAtAndSnapshot(request);
71+
expectRequestResponseToMatchSnapshot(request);
7272
}
7373

7474
@Test
@@ -82,7 +82,7 @@ public void test_trip_planning_with_walk_only_stop() {
8282
.withTo(p2)
8383
.buildRequest();
8484

85-
expectArriveByToMatchDepartAtAndSnapshot(request);
85+
expectRequestResponseToMatchSnapshot(request);
8686
}
8787

8888
@Test
@@ -121,7 +121,7 @@ public void test_trip_planning_with_transit_stop() {
121121
.withTo(p3)
122122
.buildRequest();
123123

124-
expectArriveByToMatchDepartAtAndSnapshot(request);
124+
expectRequestResponseToMatchSnapshot(request);
125125
}
126126

127127
@Test
@@ -133,6 +133,6 @@ public void test_trip_planning_with_transit_stop_collection() {
133133
.withTo(p3)
134134
.buildRequest();
135135

136-
expectArriveByToMatchDepartAtAndSnapshot(request);
136+
expectRequestResponseToMatchSnapshot(request);
137137
}
138138
}

0 commit comments

Comments
 (0)