From 57c209e3c285c305d446f71fc3c1adac5eb03d38 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Thu, 27 Jun 2024 14:51:15 -0400 Subject: [PATCH 1/6] test(CheckMonitoredTrip): Remove unused helper func. --- .../jobs/CheckMonitoredTripTest.java | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/src/test/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTripTest.java b/src/test/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTripTest.java index 0a7cfa866..8b4710996 100644 --- a/src/test/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTripTest.java +++ b/src/test/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTripTest.java @@ -1,5 +1,6 @@ package org.opentripplanner.middleware.tripmonitor.jobs; +import com.fasterxml.jackson.core.JsonProcessingException; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; @@ -209,13 +210,6 @@ private static Stream createDelayNotificationTestCases() { ); } - /** - * Convenience method for creating a CheckMonitoredTrip instance with the default journey state. - */ - private static CheckMonitoredTrip createCheckMonitoredTrip() throws Exception { - return createCheckMonitoredTrip(OtpTestUtils.createDefaultJourneyState(), null); - } - /** * Convenience method for creating a CheckMonitoredTrip instance with the default journey state. */ @@ -580,19 +574,26 @@ void canMakeOTPRequestAndResolveNoLongerPossibleTrip() throws Exception { } @Test - void shouldReportOneTimeTripInPastAsCompleted() throws CloneNotSupportedException { + void shouldReportOneTimeTripInPastAsCompleted() throws CloneNotSupportedException, JsonProcessingException { MonitoredTrip trip = makeMonitoredTripFromNow(-900, -300); trip.journeyState.matchingItinerary = trip.itinerary; trip.journeyState.tripStatus = TripStatus.TRIP_ACTIVE; - new CheckMonitoredTrip(trip).checkOtpAndUpdateTripStatus(); + // Build fake OTP response, using an existing one as template + OtpResponse otpResponse = OtpTestUtils.OTP_DISPATCHER_PLAN_RESPONSE.getResponse(); + + new CheckMonitoredTrip(trip, () -> otpResponse).checkOtpAndUpdateTripStatus(); assertEquals(TripStatus.PAST_TRIP, trip.journeyState.tripStatus); } @Test - void shouldReportOneTimeTripInPastWithTrackingAsActive() throws CloneNotSupportedException { + void shouldReportOneTimeTripInPastWithTrackingAsActive() throws CloneNotSupportedException, JsonProcessingException { MonitoredTrip trip = createPastActiveTripWithTrackedJourney(); - new CheckMonitoredTrip(trip).checkOtpAndUpdateTripStatus(); + + // Build fake OTP response, using an existing one as template + OtpResponse otpResponse = OtpTestUtils.OTP_DISPATCHER_PLAN_RESPONSE.getResponse(); + + new CheckMonitoredTrip(trip, () -> otpResponse).checkOtpAndUpdateTripStatus(); assertEquals(TripStatus.TRIP_ACTIVE, trip.journeyState.tripStatus); } @@ -644,7 +645,10 @@ void shouldReportRecurringTripInstanceInPastWithTrackingAsActive() throws Except setRecurringTodayAndTomorrow(trip); String todayFormatted = trip.journeyState.targetDate; - CheckMonitoredTrip check = new CheckMonitoredTrip(trip); + // Build fake OTP response, using an existing one as template + OtpResponse otpResponse = OtpTestUtils.OTP_DISPATCHER_PLAN_RESPONSE.getResponse(); + + CheckMonitoredTrip check = new CheckMonitoredTrip(trip, () -> otpResponse); check.shouldSkipMonitoredTripCheck(false); check.checkOtpAndUpdateTripStatus(); // Trip should remain active, and the target date should still be "today". From fc1d1db25bf5063f477c6f93353574f46599155c Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Thu, 27 Jun 2024 15:14:20 -0400 Subject: [PATCH 2/6] test(CheckMonitoredTrip): Extract mock OTP response provider. --- .../jobs/CheckMonitoredTripTest.java | 33 ++++++++++--------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/src/test/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTripTest.java b/src/test/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTripTest.java index 8b4710996..9ce8cafca 100644 --- a/src/test/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTripTest.java +++ b/src/test/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTripTest.java @@ -88,6 +88,16 @@ public void tearDownAfterTest() { DateTimeUtils.useSystemDefaultClockAndTimezone(); } + /** Provides a mock OTP 'plan' response */ + public OtpResponse mockOtpPlanResponse() { + try { + // Setup an OTP mock response in order to trigger some of the monitor checks. + return OtpTestUtils.OTP_DISPATCHER_PLAN_RESPONSE.getResponse(); + } catch (JsonProcessingException e) { + throw new RuntimeException(e); + } + } + /** * To run this trip, change the env.yml config values for OTP_API_ROOT * (and OTP_PLAN_ENDPOINT) to a valid OTP server. @@ -107,7 +117,7 @@ void canMonitorTrip() throws Exception { LOG.info("Created trip {}", monitoredTrip.id); // Setup an OTP mock response in order to trigger some of the monitor checks. - OtpResponse mockResponse = OtpTestUtils.OTP_DISPATCHER_PLAN_RESPONSE.getResponse(); + OtpResponse mockResponse = mockOtpPlanResponse(); Itinerary mockMondayJune15Itinerary = mockResponse.plan.itineraries.get(0); // parse original itinerary date/time and then update mock itinerary to occur on Monday June 15 @@ -131,7 +141,7 @@ void canMonitorTrip() throws Exception { ); // Next, run a monitor trip check from the new monitored trip using the simulated response. - CheckMonitoredTrip checkMonitoredTrip = new CheckMonitoredTrip(monitoredTrip, () -> mockResponse); + CheckMonitoredTrip checkMonitoredTrip = new CheckMonitoredTrip(monitoredTrip, this::mockOtpPlanResponse); checkMonitoredTrip.run(); // Assert that there is one notification generated during check. // TODO: Improve assertions to use snapshots. @@ -574,26 +584,20 @@ void canMakeOTPRequestAndResolveNoLongerPossibleTrip() throws Exception { } @Test - void shouldReportOneTimeTripInPastAsCompleted() throws CloneNotSupportedException, JsonProcessingException { + void shouldReportOneTimeTripInPastAsCompleted() throws CloneNotSupportedException { MonitoredTrip trip = makeMonitoredTripFromNow(-900, -300); trip.journeyState.matchingItinerary = trip.itinerary; trip.journeyState.tripStatus = TripStatus.TRIP_ACTIVE; - // Build fake OTP response, using an existing one as template - OtpResponse otpResponse = OtpTestUtils.OTP_DISPATCHER_PLAN_RESPONSE.getResponse(); - - new CheckMonitoredTrip(trip, () -> otpResponse).checkOtpAndUpdateTripStatus(); + new CheckMonitoredTrip(trip, this::mockOtpPlanResponse).checkOtpAndUpdateTripStatus(); assertEquals(TripStatus.PAST_TRIP, trip.journeyState.tripStatus); } @Test - void shouldReportOneTimeTripInPastWithTrackingAsActive() throws CloneNotSupportedException, JsonProcessingException { + void shouldReportOneTimeTripInPastWithTrackingAsActive() throws CloneNotSupportedException { MonitoredTrip trip = createPastActiveTripWithTrackedJourney(); - // Build fake OTP response, using an existing one as template - OtpResponse otpResponse = OtpTestUtils.OTP_DISPATCHER_PLAN_RESPONSE.getResponse(); - - new CheckMonitoredTrip(trip, () -> otpResponse).checkOtpAndUpdateTripStatus(); + new CheckMonitoredTrip(trip, this::mockOtpPlanResponse).checkOtpAndUpdateTripStatus(); assertEquals(TripStatus.TRIP_ACTIVE, trip.journeyState.tripStatus); } @@ -645,10 +649,7 @@ void shouldReportRecurringTripInstanceInPastWithTrackingAsActive() throws Except setRecurringTodayAndTomorrow(trip); String todayFormatted = trip.journeyState.targetDate; - // Build fake OTP response, using an existing one as template - OtpResponse otpResponse = OtpTestUtils.OTP_DISPATCHER_PLAN_RESPONSE.getResponse(); - - CheckMonitoredTrip check = new CheckMonitoredTrip(trip, () -> otpResponse); + CheckMonitoredTrip check = new CheckMonitoredTrip(trip, this::mockOtpPlanResponse); check.shouldSkipMonitoredTripCheck(false); check.checkOtpAndUpdateTripStatus(); // Trip should remain active, and the target date should still be "today". From a1f888d5d8b2d4e43d4fd99f1738b8bcbf994c4c Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Thu, 27 Jun 2024 15:29:20 -0400 Subject: [PATCH 3/6] test(CheckMonitoredTrip): Run canMonitorTrip test without live OTP server. --- .../tripmonitor/jobs/CheckMonitoredTripTest.java | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/test/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTripTest.java b/src/test/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTripTest.java index 9ce8cafca..411af68af 100644 --- a/src/test/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTripTest.java +++ b/src/test/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTripTest.java @@ -23,7 +23,6 @@ import org.opentripplanner.middleware.otp.response.OtpResponse; import org.opentripplanner.middleware.persistence.Persistence; import org.opentripplanner.middleware.tripmonitor.TripStatus; -import org.opentripplanner.middleware.utils.ConfigUtils; import org.opentripplanner.middleware.utils.DateTimeUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -49,7 +48,6 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.junit.jupiter.api.Assumptions.assumeTrue; import static org.opentripplanner.middleware.tripmonitor.jobs.CheckMonitoredTripBasicTest.makeMonitoredTripFromNow; import static org.opentripplanner.middleware.tripmonitor.jobs.CheckMonitoredTripBasicTest.setRecurringTodayAndTomorrow; @@ -104,9 +102,6 @@ public OtpResponse mockOtpPlanResponse() { */ @Test void canMonitorTrip() throws Exception { - // Do not run this test in a CI environment because it requires a live OTP server - // FIXME: Add live otp server to e2e tests. - assumeTrue(!ConfigUtils.isRunningCi && OtpMiddlewareTestEnvironment.IS_END_TO_END); MonitoredTrip monitoredTrip = new MonitoredTrip(OtpTestUtils.sendSamplePlanRequest()); monitoredTrip.updateAllDaysOfWeek(true); monitoredTrip.userId = user.id; @@ -141,7 +136,7 @@ void canMonitorTrip() throws Exception { ); // Next, run a monitor trip check from the new monitored trip using the simulated response. - CheckMonitoredTrip checkMonitoredTrip = new CheckMonitoredTrip(monitoredTrip, this::mockOtpPlanResponse); + CheckMonitoredTrip checkMonitoredTrip = new CheckMonitoredTrip(monitoredTrip, () -> mockResponse); checkMonitoredTrip.run(); // Assert that there is one notification generated during check. // TODO: Improve assertions to use snapshots. From 28a2217be611cb8e395f6e01279d30a593fba563 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Thu, 27 Jun 2024 15:39:52 -0400 Subject: [PATCH 4/6] test(CheckMonitoredTrip): Remove mock OTP server requirements. --- .../tripmonitor/jobs/CheckMonitoredTripTest.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/test/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTripTest.java b/src/test/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTripTest.java index 411af68af..a85b2a69e 100644 --- a/src/test/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTripTest.java +++ b/src/test/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTripTest.java @@ -27,7 +27,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.io.IOException; import java.time.Instant; import java.time.ZonedDateTime; import java.time.format.DateTimeFormatter; @@ -67,8 +66,7 @@ public class CheckMonitoredTripTest extends OtpMiddlewareTestEnvironment { .withMinute(0); @BeforeAll - public static void setup() throws IOException { - OtpTestUtils.mockOtpServer(); + public static void setup() { user = PersistenceTestUtils.createUser("user@example.com"); } @@ -82,7 +80,6 @@ public static void tearDown() { @AfterEach public void tearDownAfterTest() { - OtpTestUtils.resetOtpMocks(); DateTimeUtils.useSystemDefaultClockAndTimezone(); } @@ -102,9 +99,13 @@ public OtpResponse mockOtpPlanResponse() { */ @Test void canMonitorTrip() throws Exception { - MonitoredTrip monitoredTrip = new MonitoredTrip(OtpTestUtils.sendSamplePlanRequest()); + MonitoredTrip monitoredTrip = PersistenceTestUtils.createMonitoredTrip( + user.id, + OtpTestUtils.OTP_DISPATCHER_PLAN_RESPONSE.clone(), + false, + OtpTestUtils.createDefaultJourneyState() + ); monitoredTrip.updateAllDaysOfWeek(true); - monitoredTrip.userId = user.id; monitoredTrip.tripName = "My Morning Commute"; monitoredTrip.itineraryExistence = new ItineraryExistence(); monitoredTrip.itineraryExistence.monday = new ItineraryExistence.ItineraryExistenceResult(); From 831de114bbfd57afbfbee5c34c220c7cf2c91538 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Thu, 27 Jun 2024 15:50:32 -0400 Subject: [PATCH 5/6] test(CheckMonitoredTrip): Remove null otpResponseProvider case. --- .../middleware/tripmonitor/jobs/CheckMonitoredTripTest.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/test/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTripTest.java b/src/test/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTripTest.java index a85b2a69e..e14957a20 100644 --- a/src/test/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTripTest.java +++ b/src/test/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTripTest.java @@ -160,7 +160,7 @@ void testDelayNotifications( journeyState.baselineDepartureTimeEpochMillis += previousDelayMillis; journeyState.baselineArrivalTimeEpochMillis += previousDelayMillis; - CheckMonitoredTrip check = createCheckMonitoredTrip(journeyState, null); + CheckMonitoredTrip check = createCheckMonitoredTrip(journeyState, this::mockOtpPlanResponse); check.matchingItinerary.offsetTimes(TimeUnit.MILLISECONDS.convert(minutesLate, TimeUnit.MINUTES)); NotificationType[] notificationTypes = new NotificationType[] { @@ -235,9 +235,7 @@ private static CheckMonitoredTrip createCheckMonitoredTrip(JourneyState journeyS false, journeyState ); - CheckMonitoredTrip checkMonitoredTrip = otpResponseProvider != null - ? new CheckMonitoredTrip(monitoredTrip, otpResponseProvider) - : new CheckMonitoredTrip(monitoredTrip); + CheckMonitoredTrip checkMonitoredTrip = new CheckMonitoredTrip(monitoredTrip, otpResponseProvider); checkMonitoredTrip.matchingItinerary = OtpTestUtils.createDefaultItinerary(); return checkMonitoredTrip; } From eced197c3160b85edf2d3e95829dca6567f4514b Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Thu, 27 Jun 2024 16:32:18 -0400 Subject: [PATCH 6/6] test(CheckMonitoredTrip): Inline mockOtpPlanResponse and remove redundant code. --- .../jobs/CheckMonitoredTripTest.java | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/test/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTripTest.java b/src/test/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTripTest.java index e14957a20..788abc5c2 100644 --- a/src/test/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTripTest.java +++ b/src/test/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTripTest.java @@ -105,9 +105,6 @@ void canMonitorTrip() throws Exception { false, OtpTestUtils.createDefaultJourneyState() ); - monitoredTrip.updateAllDaysOfWeek(true); - monitoredTrip.tripName = "My Morning Commute"; - monitoredTrip.itineraryExistence = new ItineraryExistence(); monitoredTrip.itineraryExistence.monday = new ItineraryExistence.ItineraryExistenceResult(); Persistence.monitoredTrips.create(monitoredTrip); LOG.info("Created trip {}", monitoredTrip.id); @@ -140,7 +137,6 @@ void canMonitorTrip() throws Exception { CheckMonitoredTrip checkMonitoredTrip = new CheckMonitoredTrip(monitoredTrip, () -> mockResponse); checkMonitoredTrip.run(); // Assert that there is one notification generated during check. - // TODO: Improve assertions to use snapshots. Assertions.assertEquals(1, checkMonitoredTrip.notifications.size()); // Clear the created trip. PersistenceTestUtils.deleteMonitoredTrip(monitoredTrip); @@ -366,8 +362,9 @@ static List createSkipTripTestCases() throws Exception { @Test void canMakeOTPRequestAndUpdateMatchingItineraryForPreviouslyUnmatchedItinerary() throws Exception { // create an OTP mock to return - OtpResponse mockWeekdayResponse = OtpTestUtils.OTP_DISPATCHER_PLAN_RESPONSE.getResponse(); - // create a mock monitored trip and CheckMonitorTrip instance + OtpResponse mockWeekdayResponse = mockOtpPlanResponse(); + // create a mock monitored trip and CheckMonitorTrip instance. + // Note that the response below gets modified from the original mockOtpPlanResponse. CheckMonitoredTrip mockCheckMonitoredTrip = createCheckMonitoredTrip(() -> mockWeekdayResponse); MonitoredTrip mockTrip = mockCheckMonitoredTrip.trip; Persistence.monitoredTrips.create(mockTrip); @@ -430,8 +427,9 @@ void canMakeOTPRequestAndUpdateMatchingItineraryForPreviouslyUnmatchedItinerary( @Test void canMakeOTPRequestAndResolveUnmatchedItinerary() throws Exception { // create an OTP mock to return - OtpResponse mockWeekdayResponse = OtpTestUtils.OTP_DISPATCHER_PLAN_RESPONSE.getResponse(); + OtpResponse mockWeekdayResponse = mockOtpPlanResponse(); // create a mock monitored trip and CheckMonitorTrip instance + // Note that the response below gets modified from the original mockOtpPlanResponse. CheckMonitoredTrip mockCheckMonitoredTrip = createCheckMonitoredTrip(() -> mockWeekdayResponse); MonitoredTrip mockTrip = mockCheckMonitoredTrip.trip; Persistence.monitoredTrips.create(mockTrip); @@ -506,8 +504,9 @@ void canMakeOTPRequestAndResolveUnmatchedItinerary() throws Exception { @Test void canMakeOTPRequestAndResolveNoLongerPossibleTrip() throws Exception { // create an OTP mock to return - OtpResponse mockWeekdayResponse = OtpTestUtils.OTP_DISPATCHER_PLAN_RESPONSE.getResponse(); + OtpResponse mockWeekdayResponse = mockOtpPlanResponse(); // create a mock monitored trip and CheckMonitorTrip instance + // Note that the response below gets modified from the original mockOtpPlanResponse. CheckMonitoredTrip mockCheckMonitoredTrip = createCheckMonitoredTrip(() -> mockWeekdayResponse); MonitoredTrip mockTrip = mockCheckMonitoredTrip.trip; Persistence.monitoredTrips.create(mockTrip); @@ -626,10 +625,11 @@ void shouldReportRecurringTripInstanceInPastAsUpcoming() throws Exception { setRecurringTodayAndTomorrow(trip); // Build fake OTP response, using an existing one as template - OtpResponse otpResponse = OtpTestUtils.OTP_DISPATCHER_PLAN_RESPONSE.getResponse(); + OtpResponse otpResponse = mockOtpPlanResponse(); Itinerary adjustedItinerary = trip.itinerary.clone(); otpResponse.plan.itineraries = List.of(adjustedItinerary); + // Note that the response below gets modified from the original mockOtpPlanResponse. CheckMonitoredTrip check = new CheckMonitoredTrip(trip, () -> otpResponse); check.shouldSkipMonitoredTripCheck(false); check.checkOtpAndUpdateTripStatus();