-
Notifications
You must be signed in to change notification settings - Fork 5.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add slimmed-down Selenium project code #596
base: master
Are you sure you want to change the base?
Changes from 1 commit
56c2360
eb7db86
648edc9
32b9670
79a06fc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
# Modern Web Automation With Python and Selenium | ||
|
||
This repository contains the module `bandcamp`, which is the sample app built in the Real Python tutorial [Modern Web Automation With Python and Selenium](https://realpython.com/modern-web-automation-with-python-and-selenium/). | ||
|
||
## Installation and Setup | ||
|
||
Create and activate a [Python virtual environment](https://realpython.com/python-virtual-environments-a-primer/). | ||
|
||
Then, install the requirements: | ||
|
||
```sh | ||
(venv) $ python -m pip install -r requirements.txt | ||
``` | ||
|
||
The only direct dependency for this project is [Selenium](https://selenium-python.readthedocs.io/). You should use a Python version of at least 3.8. | ||
|
||
## Run the Bandcamp Discover Player | ||
|
||
To run the music placer, navigate to the `src/` folder, then execute the module from your command-line: | ||
|
||
```sh | ||
(venv) $ cd src/ | ||
(venv) $ python -m bandcamp | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔧 Technical Recommendation That doesn't work:
A better option would be to install the package into the venv first. That way, it won't matter where you'll run the module from:
|
||
``` | ||
|
||
You'll see a text-based user interface that allows you to interact with the music player: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤝 Handholding It seems that you implicitly assume there's a driver for Firefox installed and configured for this to run. I don't have one, and this is what I got:
Probably better to give a heads-up or provide instructions on what needs to be installed before. |
||
|
||
``` | ||
Type: [play <track number>], [tracks], [more], [exit] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Flagging the message (I'll explain why in another comment.) |
||
> | ||
``` | ||
|
||
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`. | ||
|
||
## About the Authors | ||
|
||
Martin Breuss - Email: [email protected] | ||
Bartosz Zaczyński - Email: [email protected] | ||
|
||
## License | ||
|
||
Distributed under the MIT license. See ``LICENSE`` for more information. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
[build-system] | ||
requires = ["setuptools", "wheel"] | ||
build-backend = "setuptools.build_meta" | ||
|
||
[project] | ||
name = "bandcamp_player" | ||
version = "0.1.0" | ||
requires-python = ">=3.8" | ||
description = "A web player for Bandcamp using Selenium" | ||
authors = [ | ||
{ name = "Martin Breuss", email = "[email protected]" }, | ||
{ name = "Bartosz Zaczyński", email = "[email protected]" }, | ||
] | ||
dependencies = [ | ||
"selenium", | ||
] | ||
[project.scripts] | ||
bandcamp-player = "bandcamp.__main__:main" | ||
|
||
[tool.setuptools.packages.find] | ||
where = ["src"] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔧 Technical Recommendation These two lines are redundant, as setuptools will automatically discover Python packages following the src-layout. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
appdirs==1.4.4 | ||
attrs==24.2.0 | ||
certifi==2024.7.4 | ||
h11==0.14.0 | ||
idna==3.7 | ||
jedi==0.19.1 | ||
outcome==1.3.0.post0 | ||
parso==0.8.4 | ||
prompt_toolkit==3.0.47 | ||
ptpython==3.0.29 | ||
Pygments==2.18.0 | ||
PySocks==1.7.1 | ||
selenium==4.23.1 | ||
sniffio==1.3.1 | ||
sortedcontainers==2.4.0 | ||
trio==0.26.1 | ||
trio-websocket==0.11.1 | ||
typing_extensions==4.12.2 | ||
urllib3==2.2.2 | ||
wcwidth==0.2.13 | ||
websocket-client==1.8.0 | ||
wsproto==1.2.0 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔧 Technical Recommendation Are you sure you haven't accidentally installed any extra packages besides Selenium? Here's what my requirements.txt file looks like:
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,11 @@ | ||||||
from bandcamp.app.tui import TUI | ||||||
|
||||||
|
||||||
def main(): | ||||||
"""Provides the main entry point for the app.""" | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔧 Technical Recommendation According to the widely-adopted docstring conventions (PEP 257), this sort of comments should use the imperative form, e.g.:
Suggested change
|
||||||
tui = TUI() | ||||||
tui.interact() | ||||||
|
||||||
|
||||||
if __name__ == "__main__": | ||||||
main() | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔧 Technical Recommendation Do we need those two lines here? Since you defined the $ bandcamp-player Unless, you do want to have two ways of running your TUI:
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,45 @@ | ||||||
from selenium.webdriver import Firefox | ||||||
from selenium.webdriver.firefox.options import Options | ||||||
|
||||||
from bandcamp.web.element import TrackElement | ||||||
from bandcamp.web.page import HomePage | ||||||
|
||||||
BANDCAMP_FRONTPAGE = "https://bandcamp.com/" | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔧 Technical Recommendation Since this value represents a URL rather than, say, a page object for the frontpage, let's be a bit more explicit here:
Suggested change
|
||||||
|
||||||
|
||||||
class Player: | ||||||
"""Plays tracks from Bandcamp's Discover section.""" | ||||||
|
||||||
def __init__(self) -> None: | ||||||
self._driver = self._set_up_driver() | ||||||
self.home = HomePage(self._driver) | ||||||
self.discover = self.home.discover_tracklist | ||||||
self._current_track = TrackElement( | ||||||
self.home.discover_tracklist.available_tracks[0], self._driver | ||||||
) | ||||||
|
||||||
def __enter__(self): | ||||||
return self | ||||||
|
||||||
def __exit__(self, exc_type, exc_value, exc_tb): | ||||||
"""Closes the headless browser.""" | ||||||
self._driver.close() | ||||||
|
||||||
def play(self, track_number=None): | ||||||
"""Plays the first track, or one of the available numbered tracks.""" | ||||||
if track_number: | ||||||
self._current_track = TrackElement( | ||||||
self.home.discover_tracklist.available_tracks[ | ||||||
track_number - 1 | ||||||
], | ||||||
self._driver, | ||||||
) | ||||||
self._current_track.play() | ||||||
|
||||||
def _set_up_driver(self): | ||||||
"""Creates a headless browser pointing to Bandcamp.""" | ||||||
options = Options() | ||||||
options.add_argument("--headless") | ||||||
browser = Firefox(options=options) | ||||||
browser.get(BANDCAMP_FRONTPAGE) | ||||||
return browser |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,66 @@ | ||||||
from bandcamp.app.player import Player | ||||||
|
||||||
|
||||||
class TUI: | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔧 Technical Recommendation Since this class has no state, only behaviors, there's no need to define it. I'd strongly suggest replacing your class methods with simple top-level functions. It'll make the code less Java-esque ☕ |
||||||
"""Provides a text-based user interface for a Bandcamp music player.""" | ||||||
|
||||||
COLUMN_WIDTH = CW = 30 | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice aliasing! |
||||||
|
||||||
def interact(self): | ||||||
"""Controls the player through user interactions.""" | ||||||
with Player() as player: | ||||||
while True: | ||||||
print( | ||||||
"\nType: [play <track number>], [tracks], [more], [exit]" | ||||||
) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔧 Technical Recommendation A quick note on the syntax used in this help message:
It's a widespread convention to use the square brackets to denote optional attributes or values. Therefore, a more appropriate version of this message would look as follows:
It clearly communicates that the track number is optional for the |
||||||
command = input("> ").strip().lower() | ||||||
|
||||||
if command.startswith("play"): | ||||||
try: | ||||||
track_number = int(command.split()[1]) | ||||||
self.play(player, track_number) | ||||||
except IndexError: # Play first track. | ||||||
self.play(player) | ||||||
except ValueError: | ||||||
print("Please provide a valid track number.") | ||||||
elif command == "tracks": | ||||||
self.tracks(player) | ||||||
elif command == "more": | ||||||
player.discover.load_more() | ||||||
self.tracks(player) | ||||||
elif command == "exit": | ||||||
print("Exiting the player...") | ||||||
break | ||||||
else: | ||||||
print("Unknown command. Try again.") | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔧 Technical Recommendation I'm inclined to suggest pattern matching here to simplify the logic. Parsing user input is the perfect use case for structural pattern matching: match input("> ").strip().lower().split():
case ["play"]:
self.play(player)
case ["play", track]:
try:
track_number = int(track)
self.play(player, track_number)
except ValueError:
print("Please provide a valid track number.")
case ["tracks"]:
self.tracks(player)
case ["more"]:
player.discover.load_more()
self.tracks(player)
case ["exit"]:
print("Exiting the player...")
break
case _:
print("Unknown command. Try again.") It's used as an example in one of the PEPs for pattern matching, and I also use a few of similar examples in my tutorial. This would make an opportunity to include a link to the said tutorial. At the same time, I'll understand if you want to push back on this one, so no worries if you don't like this idea. |
||||||
|
||||||
def play(self, player, track_number=None): | ||||||
"""Plays a track and shows info about the track.""" | ||||||
player.play(track_number) | ||||||
print(player._current_track._get_track_info()) | ||||||
|
||||||
def tracks(self, player): | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔧 Technical Recommendation Can we use a more descriptive name for this method, such as this?
Suggested change
It would render the dostring that follows less important 😉 |
||||||
"""Displays information about the currently playable tracks.""" | ||||||
header = ( | ||||||
f"{'#':<5} {'Album':<{self.CW}} " | ||||||
f"{'Artist':<{self.CW}} " | ||||||
f"{'Genre':<{self.CW}}" | ||||||
) | ||||||
print(header) | ||||||
print("-" * 100) | ||||||
for track_number, track in enumerate( | ||||||
player.discover.available_tracks, start=1 | ||||||
): | ||||||
album, artist, *genre = track.text.split("\n") | ||||||
album = self._truncate(album, self.CW) | ||||||
artist = self._truncate(artist, self.CW) | ||||||
genre = self._truncate(genre[0], self.CW) if genre else "" | ||||||
print( | ||||||
f"{track_number:<5} {album:<{self.CW}} " | ||||||
f"{artist:<{self.CW}} {genre:<{self.CW}}" | ||||||
) | ||||||
|
||||||
@staticmethod | ||||||
def _truncate(text, width): | ||||||
"""Truncates track information.""" | ||||||
return text[: width - 3] + "..." if len(text) > width else text |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
from dataclasses import dataclass | ||
|
||
from selenium.webdriver.remote.webdriver import WebDriver | ||
from selenium.webdriver.remote.webelement import WebElement | ||
from selenium.webdriver.support.wait import WebDriverWait | ||
|
||
MAX_WAIT_SECONDS = 10.0 | ||
|
||
|
||
@dataclass | ||
class Track: | ||
album: str | ||
artist: str | ||
genre: str | ||
url: str | ||
|
||
|
||
class WebPage: | ||
def __init__(self, driver: WebDriver) -> None: | ||
self._driver = driver | ||
self._wait = WebDriverWait(driver, MAX_WAIT_SECONDS) | ||
|
||
|
||
class WebComponent(WebPage): | ||
def __init__(self, parent: WebElement, driver: WebDriver) -> None: | ||
super().__init__(driver) | ||
self._parent = parent |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
from selenium.webdriver.remote.webdriver import WebDriver | ||
from selenium.webdriver.remote.webelement import WebElement | ||
|
||
from bandcamp.web.base import Track, WebComponent | ||
from bandcamp.web.locators import HomePageLocatorMixin, TrackLocatorMixin | ||
|
||
|
||
class TrackElement(WebComponent, TrackLocatorMixin): | ||
"""Models a playable track in Bandcamp's Discover section.""" | ||
|
||
def play(self) -> None: | ||
"""Plays the track.""" | ||
if not self.is_playing(): | ||
self._get_play_button().click() | ||
self._wait.until(lambda _: self.is_playing()) | ||
|
||
def is_playing(self) -> bool: | ||
return "playing" in self._get_play_button().get_attribute("class") | ||
|
||
def _get_track_info(self) -> Track: | ||
"""Creates a representation of the track's relevant information.""" | ||
full_url = self._parent.find_element(*self.ALBUM).get_attribute("href") | ||
# Cut off the referrer query parameter | ||
clean_url = full_url.split("?")[0] if full_url else "" | ||
return Track( | ||
album=self._parent.find_element(*self.ALBUM).text, | ||
artist=self._parent.find_element(*self.ARTIST).text, | ||
genre=self._parent.find_element(*self.GENRE).text, | ||
url=clean_url, | ||
) | ||
|
||
def _get_play_button(self): | ||
return self._parent.find_element(*self.PLAY_BUTTON) | ||
|
||
|
||
class DiscoverTrackList(WebComponent, HomePageLocatorMixin): | ||
"""Models the track list in Bandcamp's Discover section.""" | ||
|
||
def __init__(self, parent: WebElement, driver: WebDriver = None) -> None: | ||
super().__init__(parent, driver) | ||
self.available_tracks = self._get_available_tracks() | ||
|
||
def load_more(self) -> None: | ||
"""Loads additional tracks in the Discover section.""" | ||
self._get_next_page_button().click() | ||
self.available_tracks = self._get_available_tracks() | ||
|
||
def _get_available_tracks(self) -> list: | ||
"""Finds all currently available tracks in the Discover section.""" | ||
all_tracks = self._driver.find_elements(*self.TRACK) | ||
return [track for track in all_tracks if track.is_displayed()] | ||
|
||
def _get_next_page_button(self): | ||
"""Locates and returns the 'Next' button that loads more results.""" | ||
return self._driver.find_elements(*self.PAGINATION_BUTTON)[-1] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
from selenium.webdriver.common.by import By | ||
|
||
|
||
class HomePageLocatorMixin: | ||
DISCOVER_RESULTS = (By.CLASS_NAME, "discover-results") | ||
TRACK = (By.CLASS_NAME, "discover-item") | ||
PAGINATION_BUTTON = (By.CLASS_NAME, "item-page") | ||
|
||
|
||
class TrackLocatorMixin: | ||
PLAY_BUTTON = (By.CSS_SELECTOR, "a") | ||
ALBUM = (By.CLASS_NAME, "item-title") | ||
GENRE = (By.CLASS_NAME, "item-genre") | ||
ARTIST = (By.CLASS_NAME, "item-artist") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔧 Technical Recommendation I really like your approach of using the "mixin" classes to store various element locators. It's a clever way to keep them separate but close at the same time 👏 That being said, I'm not convinced we should call these classes mixins. A mixin class encapsulates behavior, and it typically depends on some attributes defined in the target class, so a mixin can't exist on its own—it needs to be "mixed-in" with another ingredient. In this case, your classes are just data containers or convenient namespaces for a bunch of constants that can stand on their own. So, I'd suggest to drop the "Mixin" suffix from their names. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
from selenium.webdriver.remote.webdriver import WebDriver | ||
|
||
from bandcamp.web.base import WebPage | ||
from bandcamp.web.element import DiscoverTrackList | ||
from bandcamp.web.locators import HomePageLocatorMixin | ||
|
||
|
||
class HomePage(WebPage, HomePageLocatorMixin): | ||
"""Models the relevant parts of the Bandcamp home page.""" | ||
|
||
def __init__(self, driver: WebDriver) -> None: | ||
super().__init__(driver) | ||
self.discover_tracklist = DiscoverTrackList( | ||
self._driver.find_element(*self.DISCOVER_RESULTS), self._driver | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✍️ Typo