Merge branch 'jdholtz:develop' into develop

lib/reservation_monitor.py

@@ -59,7 +59,7 @@ def _monitor(self) -> None:
             with self.lock:
                 logger.debug("Lock acquired")
 
-                should_exit = self._perform_check()
+                should_exit = self._check()
                 if should_exit:
                     logger.debug("Stopping monitoring")
                     break
@@ -71,7 +71,7 @@ def _monitor(self) -> None:
             logger.debug("Lock released")
             self._smart_sleep(time_before)
 
-    def _perform_check(self) -> bool:
+    def _check(self) -> bool:
         """
         Check for reservation changes and lower fares. Returns true if future checks should not be
         performed (e.g. no more flights are scheduled to check in).
@@ -172,7 +172,7 @@ def __init__(self, config: AccountConfig, lock: multiprocessing.Lock) -> None:
         self.username = config.username
         self.password = config.password
 
-    def _perform_check(self) -> bool:
+    def _check(self) -> bool:
         """
         Check for newly booked reservations for the account. Returns true if future checks should
         not be performed.

tests/integration/test_script_set_up.py

@@ -25,7 +25,7 @@ def logger(mocker: MockerFixture) -> Iterator[logging.Logger]:
 
 @pytest.fixture(autouse=True)
 def mock_read_config(mocker: MockerFixture) -> None:
-    # Don't ever read the actually config file. Will be mocked within the test
+    # Don't ever read the actual config file. Will be mocked within the test
     # if a certain config needs to be used
     mocker.patch("pathlib.Path.read_text", side_effect=FileNotFoundError)
 

tests/unit/test_checkin_handler.py

@@ -32,7 +32,7 @@ def test_schedule_check_in_starts_a_process(self, mocker: MockerFixture) -> None
         self.handler.schedule_check_in()
 
         mock_process.return_value.start.assert_called_once()
-        assert self.handler.pid is not None
+        assert self.handler.pid is not None, "PID was not set while scheduling a check-in"
 
     def test_stop_check_in_stops_a_process_by_killing_its_pid(self, mocker: MockerFixture) -> None:
         mock_os_kill = mocker.patch("os.kill")

tests/unit/test_checkin_scheduler.py

@@ -75,15 +75,17 @@ def test_get_flights_retrieves_all_flights_under_reservation(
         mocker.patch("lib.checkin_scheduler.get_current_time", return_value=current_time)
 
         flights = self.scheduler._get_flights("flight1")
-        assert len(flights) == 2
-        assert mock_set_same_day_flight.call_count == len(flights)
+        assert len(flights) == 2, "Unexpected number of flights retrieved"
+        assert mock_set_same_day_flight.call_count == len(
+            flights
+        ), "_set_same_day_flight() not called once for every retrieved flight"
 
-    def test_get_flights_schedules_no_flights_on_request_error(self, mocker: MockerFixture) -> None:
+    def test_get_flights_retrieves_no_flights_on_request_error(self, mocker: MockerFixture) -> None:
         mocker.patch("lib.checkin_scheduler.make_request", side_effect=RequestError(""))
 
         flights = self.scheduler._get_flights("flight1")
 
-        assert len(flights) == 0
+        assert len(flights) == 0, "Flights were retrieved"
 
     def test_get_flights_does_not_retrieve_departed_flights(
         self, mocker: MockerFixture, test_flights: List[Flight]
@@ -100,7 +102,7 @@ def test_get_flights_does_not_retrieve_departed_flights(
         mocker.patch("lib.checkin_scheduler.get_current_time", return_value=current_time)
 
         flights = self.scheduler._get_flights("flight1")
-        assert len(flights) == 0
+        assert len(flights) == 0, "Departed flights were retrieved"
 
     def test_get_reservation_info_returns_reservation_info(self, mocker: MockerFixture) -> None:
         reservation_content = {"viewReservationViewPage": {"bounds": [{"test": "reservation"}]}}
@@ -109,11 +111,12 @@ def test_get_reservation_info_returns_reservation_info(self, mocker: MockerFixtu
         reservation_info = self.scheduler._get_reservation_info("flight1")
         assert reservation_info == {"bounds": [{"test": "reservation"}]}
 
-    # A reservation has flights in the past and this is the first time attempting to
-    # schedule it
     def test_get_reservation_info_sends_error_notification_when_reservation_not_found(
         self, mocker: MockerFixture
     ) -> None:
+        """
+        A reservation has flights in the past and this is the first time attempting to schedule it
+        """
         mocker.patch(
             "lib.checkin_scheduler.make_request",
             side_effect=RequestError("", json.dumps({"code": FLIGHT_IN_PAST_CODE})),
@@ -128,11 +131,13 @@ def test_get_reservation_info_sends_error_notification_when_reservation_not_foun
         mock_failed_reservation_retrieval.assert_called_once()
         assert reservation_info == {}
 
-    # A reservation is already scheduled but fails for a retrieval resulting in another error than
-    # all flights being old
     def test_get_reservation_info_sends_error_when_reservation_retrieval_fails_and_flight_scheduled(
         self, mocker: MockerFixture, test_flights: List[Flight]
     ) -> None:
+        """
+        A reservation is already scheduled but fails for a retrieval resulting in another error than
+        all flights being old
+        """
         mocker.patch("lib.checkin_scheduler.make_request", side_effect=RequestError(""))
         mock_failed_reservation_retrieval = mocker.patch.object(
             NotificationHandler, "failed_reservation_retrieval"
@@ -144,10 +149,10 @@ def test_get_reservation_info_sends_error_when_reservation_retrieval_fails_and_f
         mock_failed_reservation_retrieval.assert_called_once()
         assert reservation_info == {}
 
-    # A reservation is already scheduled and the flights are in the past
     def test_get_reservation_info_does_not_send_error_notification_when_reservation_is_old(
         self, mocker: MockerFixture, test_flights: List[Flight]
     ) -> None:
+        """A reservation is already scheduled and the flights are in the past"""
         mocker.patch(
             "lib.checkin_scheduler.make_request",
             side_effect=RequestError("", json.dumps({"code": FLIGHT_IN_PAST_CODE})),
@@ -211,7 +216,9 @@ def test_schedule_flights_schedules_all_flights(
 
         assert len(self.scheduler.flights) == 2
         assert len(self.scheduler.checkin_handlers) == 2
-        assert mock_schedule_check_in.call_count == 2
+        assert (
+            mock_schedule_check_in.call_count == 2
+        ), "schedule_check_in() was not called once for every flight"
         mock_new_flights_notification.assert_called_once_with(test_flights)
 
     def test_remove_old_flights_removes_flights_not_currently_scheduled(

tests/unit/test_config.py

@@ -451,7 +451,7 @@ def test_parse_config_sets_the_correct_config_values(self, mocker: MockerFixture
         )
 
         assert test_config.browser_path == "test/browser_path"
-        assert test_config.check_fares is False  # Make sure it calls super._parse_config
+        assert test_config.check_fares is False, "Config._parse_config() was never called"
         mock_account_config.assert_called_once_with([])
         mock_reservation_config.assert_called_once_with([])
 
@@ -485,7 +485,7 @@ def test_parse_config_sets_the_correct_config_values(self) -> None:
         test_config = AccountConfig()
         test_config._parse_config({"username": "user", "password": "pass", "check_fares": False})
 
-        assert test_config.check_fares is False  # Make sure it calls super._parse_config
+        assert test_config.check_fares is False, "Config._parse_config() was never called"
         assert test_config.username == "user"
         assert test_config.password == "pass"
 
@@ -517,7 +517,7 @@ def test_parse_config_sets_the_correct_config_values(self) -> None:
         }
         test_config._parse_config(reservation_config)
 
-        assert test_config.check_fares is False  # Make sure it calls super._parse_config
+        assert test_config.check_fares is False, "Config._parse_config() was never called"
         assert test_config.confirmation_number == "num"
         assert test_config.first_name == "first"
         assert test_config.last_name == "last"

tests/unit/test_fare_checker.py

@@ -80,11 +80,13 @@ def test_get_flight_price_gets_flight_price_matching_current_flight(
         assert price == "price"
         mock_get_matching_fare.assert_called_once_with(["fare_one", "fare_two"], "test_fare")
 
-    # This scenario should not happen because Southwest should always have a flight
-    # at the same time (as it is a scheduled flight)
     def test_get_flight_price_raises_error_when_no_matching_flights_appear(
         self, mocker: MockerFixture, test_flight: Flight
     ) -> None:
+        """
+        This scenario should not happen because Southwest should always have a flight at the same
+        time (as it is a scheduled flight). Test just in case though
+        """
         flights = [{"flightNumbers": "98"}, {"flightNumbers": "99"}]
         mocker.patch.object(
             FareChecker, "_get_matching_flights", return_value=(flights, "test_fare")
@@ -125,7 +127,7 @@ def test_get_change_flight_page_retrieves_change_flight_page(
     ) -> None:
         res_info = {
             "bounds": ["bound_one", "bound_two"],
-            "_links": {"change": {"href": "test_link", "query": "query"}},
+            "_links": {"change": {"href": "test_link", "query": "query_body"}},
         }
         flight_page = {"changeFlightPage": "test_page"}
         mock_make_request = mocker.patch("lib.fare_checker.make_request", return_value=flight_page)
@@ -139,7 +141,7 @@ def test_get_change_flight_page_retrieves_change_flight_page(
 
         call_args = mock_make_request.call_args[0]
         assert call_args[1] == BOOKING_URL + "test_link"
-        assert call_args[3] == "query"
+        assert call_args[3] == "query_body"
 
     def test_get_change_flight_page_raises_exception_when_flight_cannot_be_changed(
         self, mocker: MockerFixture
@@ -243,7 +245,7 @@ def test_check_for_companion_raises_exception_when_a_companion_is_detected(self)
         ],
     )
     def test_check_for_companion_passes_when_no_companion_exists(self, reservation: JSON) -> None:
-        # It will throw an exception if the test does not pass
+        # An exception will be thrown if the test does not pass
         self.checker._check_for_companion(reservation)
 
     def test_get_matching_fare_returns_the_correct_fare(self) -> None:

tests/unit/test_log.py

@@ -44,7 +44,7 @@ def test_init_logging_sets_verbosity_level_correctly(
     log.init_logging(logger)
 
     mock_makedirs.assert_called_once()
-    assert len(logger.handlers) == 2
+    assert len(logger.handlers) == 2, "Expected a file and console handler to be created"
     assert logger.handlers[1].level == verbosity_level
 
 

tests/unit/test_main.py

@@ -10,9 +10,9 @@
 from lib.reservation_monitor import AccountMonitor, ReservationMonitor
 
 
-# We don't actually want the config to read the file for these tests
 @pytest.fixture(autouse=True)
 def mock_config(mocker: MockerFixture) -> None:
+    """The config file shouldn't actually be read for these tests"""
     mocker.patch("lib.config.GlobalConfig._read_config")
 
 

tests/unit/test_reservation_monitor.py

@@ -59,19 +59,17 @@ def test_monitor_monitors_continuously(self, mocker: MockerFixture) -> None:
         # Since the monitor function runs in an infinite loop, throw an Exception when the
         # sleep function is called a second time to break out of the loop.
         mocker.patch.object(ReservationMonitor, "_smart_sleep", side_effect=["", StopIteration])
-        mock_perform_check = mocker.patch.object(
-            ReservationMonitor, "_perform_check", return_value=False
-        )
+        mock_check = mocker.patch.object(ReservationMonitor, "_check", return_value=False)
 
         self.monitor.config.retrieval_interval = 1
         with pytest.raises(StopIteration):
             self.monitor._monitor()
 
-        assert mock_perform_check.call_count == 2
+        assert mock_check.call_count == 2
 
-    def test_monitor_monitors_until_perform_check_says_to_exit(self, mocker: MockerFixture) -> None:
+    def test_monitor_monitors_until_check_says_to_exit(self, mocker: MockerFixture) -> None:
         mock_smart_sleep = mocker.patch.object(ReservationMonitor, "_smart_sleep")
-        mocker.patch.object(ReservationMonitor, "_perform_check", side_effect=[False, False, True])
+        mocker.patch.object(ReservationMonitor, "_check", side_effect=[False, False, True])
 
         self.monitor.config.retrieval_interval = 1
         self.monitor._monitor()
@@ -82,17 +80,15 @@ def test_monitor_monitors_once_if_retrieval_interval_is_zero(
         self, mocker: MockerFixture
     ) -> None:
         mock_smart_sleep = mocker.patch.object(ReservationMonitor, "_smart_sleep")
-        mock_perform_check = mocker.patch.object(
-            ReservationMonitor, "_perform_check", return_value=False
-        )
+        mock_check = mocker.patch.object(ReservationMonitor, "_check", return_value=False)
 
         self.monitor.config.retrieval_interval = 0
         self.monitor._monitor()
 
-        mock_perform_check.assert_called_once()
+        mock_check.assert_called_once()
         mock_smart_sleep.assert_not_called()
 
-    def test_perform_check_checks_reservations(self, mocker: MockerFixture) -> None:
+    def test_check_checks_reservations(self, mocker: MockerFixture) -> None:
         mock_refresh_headers = mocker.patch.object(CheckInScheduler, "refresh_headers")
         mock_schedule_reservations = mocker.patch.object(
             ReservationMonitor, "_schedule_reservations"
@@ -102,7 +98,7 @@ def test_perform_check_checks_reservations(self, mocker: MockerFixture) -> None:
         self.monitor.config.confirmation_number = "test_num"
         self.monitor.checkin_scheduler.flights = ["test_flight"]
 
-        should_exit = self.monitor._perform_check()
+        should_exit = self.monitor._check()
 
         assert not should_exit
         mock_refresh_headers.assert_called_once()
@@ -111,7 +107,7 @@ def test_perform_check_checks_reservations(self, mocker: MockerFixture) -> None:
         )
         mock_check_flight_fares.assert_called_once()
 
-    def test_perform_check_skips_scheduling_on_driver_timeout(self, mocker: MockerFixture) -> None:
+    def test_check_skips_scheduling_on_driver_timeout(self, mocker: MockerFixture) -> None:
         mock_refresh_headers = mocker.patch.object(
             CheckInScheduler, "refresh_headers", side_effect=DriverTimeoutError
         )
@@ -124,22 +120,20 @@ def test_perform_check_skips_scheduling_on_driver_timeout(self, mocker: MockerFi
         self.monitor.config.confirmation_number = "test_num"
         self.monitor.checkin_scheduler.flights = ["test_flight"]
 
-        should_exit = self.monitor._perform_check()
+        should_exit = self.monitor._check()
 
         assert not should_exit
         mock_refresh_headers.assert_called_once()
         mock_schedule_reservations.assert_not_called()
         mock_check_flight_fares.assert_not_called()
         mock_timeout_notif.assert_called_once()
 
-    def test_perform_check_returns_false_when_no_flights_are_scheduled(
-        self, mocker: MockerFixture
-    ) -> None:
+    def test_check_returns_false_when_no_flights_are_scheduled(self, mocker: MockerFixture) -> None:
         mocker.patch.object(CheckInScheduler, "refresh_headers")
         mocker.patch.object(ReservationMonitor, "_schedule_reservations")
         mock_check_flight_fares = mocker.patch.object(ReservationMonitor, "_check_flight_fares")
 
-        should_exit = self.monitor._perform_check()
+        should_exit = self.monitor._check()
 
         assert should_exit
         mock_check_flight_fares.assert_not_called()
@@ -227,25 +221,25 @@ def _set_up_monitor(self, mock_lock: mock.Mock, mocker: MockerFixture) -> None:
             "lib.reservation_monitor.get_current_time", return_value=datetime(1999, 12, 31)
         )
 
-    def test_perform_check_checks_account_for_reservations(self, mocker: MockerFixture) -> None:
+    def test_check_checks_account_for_reservations(self, mocker: MockerFixture) -> None:
         mocker.patch.object(AccountMonitor, "_get_reservations", return_value=([], False))
         mock_schedule_reservations = mocker.patch.object(AccountMonitor, "_schedule_reservations")
         mock_check_flight_fares = mocker.patch.object(AccountMonitor, "_check_flight_fares")
 
-        should_exit = self.monitor._perform_check()
+        should_exit = self.monitor._check()
 
         assert not should_exit
         mock_schedule_reservations.assert_called_once()
         mock_check_flight_fares.assert_called_once()
 
-    def test_perform_check_skips_scheduling_if_an_error_occurs(self, mocker: MockerFixture) -> None:
+    def test_check_skips_scheduling_if_an_error_occurs(self, mocker: MockerFixture) -> None:
         # If an error occurs, _get_reservations will return an empty list of reservations and
         # true indicating scheduling should be skipped
         mocker.patch.object(AccountMonitor, "_get_reservations", return_value=([], True))
         mock_schedule_reservations = mocker.patch.object(AccountMonitor, "_schedule_reservations")
         mock_check_flight_fares = mocker.patch.object(AccountMonitor, "_check_flight_fares")
 
-        should_exit = self.monitor._perform_check()
+        should_exit = self.monitor._check()
 
         assert not should_exit
         mock_schedule_reservations.assert_not_called()

tests/unit/test_webdriver.py

@@ -143,6 +143,7 @@ def mock_sleep(_: int) -> None:
         mocker.patch("time.sleep", side_effect=mock_sleep)
 
         self.driver._wait_for_attribute("headers_set")
+        assert call_count == 2
 
     def test_wait_for_attribute_raises_error_on_timeout(self, mocker: MockerFixture) -> None:
         mocker.patch("time.sleep")