Apply TR feedback

Co-authored-by: Bartosz <[email protected]>

Author: martin-martin

python-selenium/README.md

@@ -32,7 +32,27 @@ Type: play [<track_number>] | pause | tracks | more | exit
 >
 ```
 
-Type one of the available commands to interact with Bandcamp's Discover section through your headless browser. Listen to songs with `play`, list available tracks with `tracks`, and load more songs using `more`. You can exit the music player by typing `exit`.
+Type one of the available commands to interact with Bandcamp's Discover section through your headless browser. Listen to songs with `play`, pause the current song with `pause` and restart it with `play`. List available tracks with `tracks`, and load more songs using `more`. You can exit the music player by typing `exit`.
+
+## Troubleshooting
+
+If the music player seems to hang when you run the script, confirm whether you've correctly set up your webdriver based on the following points.
+
+### Version Compatibility
+
+Confirm that your browser and corresponding webdriver are in sync. If you followed the previous suggestion, then you should be using Firefox and geckodriver. If that doesn't work for any reason, you may need to switch browser _and_ webdriver.
+
+For example, if you're using Chrome, then you need to install ChromeDriver and it must match your Chrome version. Otherwise, you may run into errors like `SessionNotCreatedException`.
+For more details, refer to the official [ChromeDriver documentation](https://sites.google.com/chromium.org/driver/) or [geckodriver releases](https://github.com/mozilla/geckodriver/releases).
+
+### Driver Installation and Path Issues
+
+Once you've confirmed that your browser and driver match, make sure that the webdriver executable is properly installed:
+
+- **Path Configuration:** The driver must be in your system's PATH, or you need to specify its full path in your code.
+- **Permissions:** Ensure the driver file has the necessary execution permissions.
+
+If you're still running into issues executing the script, then consult the [Selenium Documentation](https://www.selenium.dev/documentation/) for additional troubleshooting tips or leave a comment on the tutorial.
 
 ## About the Authors
 

python-selenium/src/bandcamp/app/tui.py

@@ -8,9 +8,7 @@ def interact():
     """Control the player through user interactions."""
     with Player() as player:
         while True:
-            print(
-                "\nType: play [<track number>] | pause | tracks | more | exit"
-            )
+            print("\nType: play [<track number>] | pause | tracks | more | exit")
             match input("> ").strip().lower().split():
                 case ["play"]:
                     play(player)
@@ -24,12 +22,8 @@ def interact():
                     pause(player)
                 case ["tracks"]:
                     display_tracks(player)
-                case ["more"] if len(
-                    player.tracklist.available_tracks
-                ) >= MAX_TRACKS:
-                    print(
-                        "Can't load more tracks. Pick one from the track list."
-                    )
+                case ["more"] if len(player.tracklist.available_tracks) >= MAX_TRACKS:
+                    print("Can't load more tracks. Pick one from the track list.")
                 case ["more"]:
                     player.tracklist.load_more()
                     display_tracks(player)
@@ -41,9 +35,15 @@ def interact():
 
 
 def play(player, track_number=None):
-    """Play a track and shows info about the track."""
-    player.play(track_number)
-    print(player._current_track._get_track_info())
+    """Play a track and show info about the track."""
+    try:
+        player.play(track_number)
+        print(player._current_track._get_track_info())
+    except IndexError:
+        print(
+            "Please provide a valid track number. "
+            "You can list available tracks with `tracks`."
+        )
 
 
 def pause(player):
@@ -56,17 +56,13 @@ def display_tracks(player):
     header = f"{'#':<5} {'Album':<{CW}} {'Artist':<{CW}} {'Genre':<{CW}}"
     print(header)
     print("-" * 100)
-    for track_number, track in enumerate(
-        player.tracklist.available_tracks, start=1
-    ):
+    for track_number, track in enumerate(player.tracklist.available_tracks, start=1):
         if track.text:
             album, artist, *genre = track.text.split("\n")
             album = _truncate(album, CW)
             artist = _truncate(artist, CW)
             genre = _truncate(genre[0], CW) if genre else ""
-            print(
-                f"{track_number:<5} {album:<{CW}} {artist:<{CW}} {genre:<{CW}}"
-            )
+            print(f"{track_number:<5} {album:<{CW}} {artist:<{CW}} {genre:<{CW}}")
 
 
 def _truncate(text, width):

python-selenium/src/bandcamp/web/element.py

@@ -7,7 +7,7 @@
 from bandcamp.web.locators import DiscoverPageLocator, TrackLocator
 
 
-class TrackElement(WebComponent, TrackLocator):
+class TrackElement(WebComponent):
     """Model a playable track in Bandcamp's Discover section."""
 
     def play(self) -> None:
@@ -26,26 +26,26 @@ def is_playing(self) -> bool:
 
     def _get_track_info(self) -> Track:
         """Create a representation of the track's relevant information."""
-        full_url = self._parent.find_element(*self.URL).get_attribute("href")
+        full_url = self._parent.find_element(*TrackLocator.URL).get_attribute("href")
         # Cut off the referrer query parameter
         clean_url = full_url.split("?")[0] if full_url else ""
         # Some tracks don't have a genre
         try:
-            genre = self._parent.find_element(*self.GENRE).text
+            genre = self._parent.find_element(*TrackLocator.GENRE).text
         except NoSuchElementException:
             genre = ""
         return Track(
-            album=self._parent.find_element(*self.ALBUM).text,
-            artist=self._parent.find_element(*self.ARTIST).text,
+            album=self._parent.find_element(*TrackLocator.ALBUM).text,
+            artist=self._parent.find_element(*TrackLocator.ARTIST).text,
             genre=genre,
             url=clean_url,
         )
 
     def _get_play_button(self):
-        return self._parent.find_element(*self.PLAY_BUTTON)
+        return self._parent.find_element(*TrackLocator.PLAY_BUTTON)
 
 
-class DiscoverTrackList(WebComponent, DiscoverPageLocator, TrackLocator):
+class DiscoverTrackList(WebComponent):
     """Model the track list in Bandcamp's Discover section."""
 
     def __init__(self, parent: WebElement, driver: WebDriver = None) -> None:
@@ -54,31 +54,35 @@ def __init__(self, parent: WebElement, driver: WebDriver = None) -> None:
 
     def load_more(self) -> None:
         """Load additional tracks in the Discover section."""
-        view_more_button = self._driver.find_element(*self.PAGINATION_BUTTON)
+        view_more_button = self._driver.find_element(
+            *DiscoverPageLocator.PAGINATION_BUTTON
+        )
         view_more_button.click()
         # The button is disabled until all new tracks are loaded.
-        self._wait.until(EC.element_to_be_clickable(self.PAGINATION_BUTTON))
+        self._wait.until(EC.element_to_be_clickable(DiscoverPageLocator.PAGINATION_BUTTON))
         self.available_tracks = self._get_available_tracks()
 
     def _get_available_tracks(self) -> list:
         """Find all currently available tracks in the Discover section."""
         self._wait.until(
-            lambda driver: any(
-                e.is_displayed() and e.text.strip()
-                for e in driver.find_elements(*self.ITEM)
-            ),
+            self._track_text_loaded,
             message="Timeout waiting for track text to load",
         )
 
-        all_items = self._driver.find_elements(*self.ITEM)
+        all_items = self._driver.find_elements(*DiscoverPageLocator.ITEM)
         all_tracks = []
         for item in all_items:
-            if item.find_element(*self.PLAY_BUTTON):
+            if item.find_element(*TrackLocator.PLAY_BUTTON):
                 all_tracks.append(item)
 
         # Filter tracks that are displayed and have text.
         return [
-            track
-            for track in all_tracks
-            if track.is_displayed() and track.text.strip()
+            track for track in all_tracks if track.is_displayed() and track.text.strip()
         ]
+
+    def _track_text_loaded(self, driver):
+        """Check if the track text has loaded."""
+        return any(
+            e.is_displayed() and e.text.strip()
+            for e in driver.find_elements(*DiscoverPageLocator.ITEM)
+        )

python-selenium/src/bandcamp/web/page.py

@@ -5,16 +5,17 @@
 from bandcamp.web.locators import DiscoverPageLocator
 
 
-class DiscoverPage(WebPage, DiscoverPageLocator):
+class DiscoverPage(WebPage):
     """Model the relevant parts of the Bandcamp Discover page."""
 
     def __init__(self, driver: WebDriver) -> None:
         super().__init__(driver)
         self._accept_cookie_consent()
         self.discover_tracklist = DiscoverTrackList(
-            self._driver.find_element(*self.DISCOVER_RESULTS), self._driver
+            self._driver.find_element(*DiscoverPageLocator.DISCOVER_RESULTS),
+            self._driver,
         )
 
     def _accept_cookie_consent(self) -> None:
         """Accept the necessary cookie consent."""
-        self._driver.find_element(*self.COOKIE_ACCEPT_NECESSARY).click()
+        self._driver.find_element(*DiscoverPageLocator.COOKIE_ACCEPT_NECESSARY).click()