Improve testing comments and assert statements

Doc strings are now added in tests where appropriate. Also, some assert
statements now include a message to what failed to make it more clear to
the developer what exactly went wrong

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_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")