Add slimmed-down Selenium project code #596

martin-martin · 2024-10-14T13:28:43Z

Where to put new files:

New files should go into a top-level subfolder, named after the article slug. For example: my-awesome-article

How to merge your changes:

Make sure the CI code style tests all pass (+ run the automatic code formatter if necessary).
Find an RP Team member on Slack and ask them to review & approve your PR.
Once the PR has one positive ("approved") review, GitHub lets you merge the PR.
🎉

martin-martin · 2024-10-14T13:34:11Z

@bzaczynski this is my attempt to create a slimmed-down version of the project that primarily focuses on Selenium web interactions (now following POM), but also keep the basic functionality of the music player (and a reduced---but existing---separation of concerns).

I've tried to build the project in a way so that it can lead into the project that you wrote, with the idea in mind that learners could follow your SbSP as a next step if they're interested in the music player project. At the same time, I tried to keep it simple and reduced enough that learners who are only there for an intro to Selenium would get their money's worth.

Hope this works and looking forward to your thoughts!

Linking the other PR for reference: #583

bzaczynski · 2024-10-23T14:17:26Z

python-selenium/README.md

+
+## Run the Bandcamp Discover Player
+
+To run the music placer, navigate to the `src/` folder, then execute the module from your command-line:


✍️ Typo

Suggested change

To run the music placer, navigate to the `src/` folder, then execute the module from your command-line:

To run the music player, navigate to the `src/` folder, then execute the module from your command-line:

bzaczynski · 2024-10-23T14:20:02Z

python-selenium/README.md

+(venv) $ cd src/
+(venv) $ python -m bandcamp


🔧 Technical Recommendation

That doesn't work:

(venv) $ cd src/ (venv) $ python -m bandcamp/ /home/bartek/.virtualenvs/python-selenium/bin/python: No module named bandcamp/

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:

(venv) $ python -m pip install . (venv) $ python -m bandcamp/

bzaczynski

@martin-martin I left a few comments for you. Let me know what you think.

bzaczynski · 2024-10-23T14:22:04Z

python-selenium/README.md

+(venv) $ python -m bandcamp
+```
+
+You'll see a text-based user interface that allows you to interact with the music player:


🤝 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:

(venv) $ python -m bandcamp Traceback (most recent call last): ... selenium.common.exceptions.InvalidArgumentException: Message: binary is not a Firefox executable

Probably better to give a heads-up or provide instructions on what needs to be installed before.

bzaczynski · 2024-10-23T14:23:10Z

python-selenium/README.md

+You'll see a text-based user interface that allows you to interact with the music player:
+
+```
+Type: [play <track number>], [tracks], [more], [exit]


Flagging the message (I'll explain why in another comment.)

bzaczynski · 2024-10-23T14:27:00Z

python-selenium/pyproject.toml

+[tool.setuptools.packages.find]
+where = ["src"]


🔧 Technical Recommendation

These two lines are redundant, as setuptools will automatically discover Python packages following the src-layout.

bzaczynski · 2024-10-23T14:28:28Z

python-selenium/requirements.txt

+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


🔧 Technical Recommendation

Are you sure you haven't accidentally installed any extra packages besides Selenium? Here's what my requirements.txt file looks like:

attrs==24.2.0 certifi==2024.8.30 h11==0.14.0 idna==3.10 outcome==1.3.0.post0 PySocks==1.7.1 selenium==4.25.0 sniffio==1.3.1 sortedcontainers==2.4.0 trio==0.27.0 trio-websocket==0.11.1 typing_extensions==4.12.2 urllib3==2.2.3 websocket-client==1.8.0 wsproto==1.2.0

bzaczynski · 2024-10-23T14:30:49Z

python-selenium/src/bandcamp/__main__.py

+if __name__ == "__main__":
+    main()


🔧 Technical Recommendation

Do we need those two lines here? Since you defined the bandcamp-player script in pyproject.toml, you can invoke the main() function directly:

$ bandcamp-player

Unless, you do want to have two ways of running your TUI:

bandcamp-player

python -m bandcamp

bzaczynski · 2024-10-23T14:50:01Z

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

+from bandcamp.app.player import Player
+
+
+class TUI:


🔧 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 ☕

bzaczynski · 2024-10-23T14:54:12Z

python-selenium/src/bandcamp/__main__.py

+
+
+def main():
+    """Provides the main entry point for the app."""


🔧 Technical Recommendation

According to the widely-adopted docstring conventions (PEP 257), this sort of comments should use the imperative form, e.g.:

Suggested change

"""Provides the main entry point for the app."""

"""Provide the main entry point for the app."""

bzaczynski · 2024-10-23T14:58:10Z

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

+        player.play(track_number)
+        print(player._current_track._get_track_info())
+
+    def tracks(self, player):


🔧 Technical Recommendation

Can we use a more descriptive name for this method, such as this?

Suggested change

def tracks(self, player):

def display_tracks(self, player):

It would render the dostring that follows less important 😉

bzaczynski · 2024-10-23T14:58:45Z

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

+class TUI:
+    """Provides a text-based user interface for a Bandcamp music player."""
+
+    COLUMN_WIDTH = CW = 30


Nice aliasing!

bzaczynski · 2024-10-23T15:00:23Z

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

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


🔧 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.

Co-authored-by: Bartosz <bartosz.zaczynski@gmail.com>

…ials into python-selenium

bzaczynski

@martin-martin Perfecto ✨ Thanks for a quick update! Can't wait to review the rest of it 😉

Bandcamp removed the "Discover" section from their main page since we wrote this code. Now, the tracks are only available at the dedicated /discover URL. I restructured the code to target the /discover site instead, which required a couple of changes. Still, the existing POM structure was helpful :) I also (re)introduced a new "pause" option, because that's a bit easier in this new structure. Finally, I only allow loading more songs once, which gives a total of 120 songs to pick from with the default screen setting. Attempting to load more resulted in errors, I think because they'd be outside of the viewport and I didn't want to expand the code more and introduce scrolling more of them into view. Also, not sure whether that'd take earlier ones out of the viewport (etc) so I just decided not to open that can of worms for this "Intro to Selenium" tutorial. LMK if you disagree, otherwise of course you can tackle that in your longer one that builds on top of this one :)

Co-authored-by: Bartosz <bartosz.zaczynski@gmail.com>

bzaczynski

@martin-martin Hi Martin!

The code looks great overall. I only left a few minor comments for your consideration, but no explicit requests for change.

Take care!

bzaczynski · 2025-03-16T08:28:33Z

python-selenium/README.md

+
+The only direct dependency for this project is [Selenium](https://selenium-python.readthedocs.io/). You should use a Python version of at least 3.10, which is necessary to support [structural pattern matching](https://realpython.com/structural-pattern-matching/).
+
+You'll need a [Firefox Selenium driver](https://selenium-python.readthedocs.io/installation.html#drivers) called `geckodriver` to run the project as-is. Make sure to [download and install](https://github.com/mozilla/geckodriver/releases) it before running the project.


@martin-martin For some reason, I was having trouble starting the script due to problems with my Firefox browser. Your script would just hang without showing any output, leaving me a bit puzzled. However, I had a hunch where to look for the problem, so switching to Chrome fixed it for me. I'm wondering if we could give the users some heads up in the README about potential issues and how to troubleshoot them.

bzaczynski · 2025-03-16T08:30:52Z

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

+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())


@martin-martin It would be nice to include some rudimentary error handling. For example, when I request to play a track outside the valid range, the script crashes abruptly.

bzaczynski · 2025-03-16T08:34:10Z

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

+        return self._parent.find_element(*self.PLAY_BUTTON)
+
+
+class DiscoverTrackList(WebComponent, DiscoverPageLocator, TrackLocator):


@martin-martin I believe I mentioned this before, but I'm not a big fan of abusing inheritance for code reuse like here, where you inherit the constans. Class mixins are primarily about combining behaviors. When you only have data attributes, it's clearer to use composition and delegation instead.

For example, instead of this:

view_more_button = self._driver.find_element(*self.PAGINATION_BUTTON)

...you'd have that:

class DiscoverTrackList(WebComponent): # ... view_more_button = self._driver.find_element(*DiscoverPageLocator.PAGINATION_BUTTON)

While more verbose, this version tells me exactly which web component the locator belongs to, so I don't have to hunt for it in the source code.

bzaczynski · 2025-03-16T08:37:38Z

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

+        self._wait.until(
+            lambda driver: any(
+                e.is_displayed() and e.text.strip()
+                for e in driver.find_elements(*self.ITEM)
+            ),
+            message="Timeout waiting for track text to load",
+        )


@martin-martin This lambda expression is relatively dense, which is why I'd personally opt for a regular function with a descriptive name so that the code reads more easily:

def _get_available_tracks(self) -> list: self._wait.until(track_text_loaded, message="Timeout waiting for track text to load", ) # ... def track_text_loaded(driver): return any( e.is_displayed() and e.text.strip() for e in driver.find_elements(*self.ITEM) )

Co-authored-by: Bartosz <bartosz.zaczynski@gmail.com>

Add Selenium project code

56c2360

martin-martin requested a review from bzaczynski October 14, 2024 13:28

martin-martin self-assigned this Oct 14, 2024

bzaczynski reviewed Oct 23, 2024

View reviewed changes

bzaczynski requested changes Oct 23, 2024

View reviewed changes

bzaczynski and others added 4 commits October 23, 2024 17:06

Merge branch 'master' into python-selenium

eb7db86

Apply Technical Review suggestions

648edc9

Co-authored-by: Bartosz <bartosz.zaczynski@gmail.com>

Merge branch 'python-selenium' of https://github.com/realpython/mater…

32b9670

…ials into python-selenium

Merge branch 'master' into python-selenium

79a06fc

martin-martin requested a review from bzaczynski October 28, 2024 12:59

bzaczynski approved these changes Oct 28, 2024

View reviewed changes

martin-martin and others added 3 commits March 12, 2025 19:43

Re-apply old TR feedback

ae7cdf5

Co-authored-by: Bartosz <bartosz.zaczynski@gmail.com>

Fix linter error

07280d0

martin-martin requested a review from bzaczynski March 13, 2025 19:51

martin-martin and others added 2 commits March 13, 2025 20:52

Fix black formatting

a0a3391

Merge branch 'master' into python-selenium

d17eb80

bzaczynski reviewed Mar 16, 2025

View reviewed changes

martin-martin and others added 4 commits March 16, 2025 11:00

Apply TR feedback

21d505c

Co-authored-by: Bartosz <bartosz.zaczynski@gmail.com>

Fix double-exit loop

b6b18e8

Refix ruff to black

37a10cf

Merge branch 'master' into python-selenium

ee77eb2

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add slimmed-down Selenium project code #596

Add slimmed-down Selenium project code #596

martin-martin commented Oct 14, 2024

martin-martin commented Oct 14, 2024

bzaczynski Oct 23, 2024

bzaczynski Oct 23, 2024 •

edited

Loading

bzaczynski left a comment

bzaczynski Oct 23, 2024

bzaczynski Oct 23, 2024

bzaczynski Oct 23, 2024

bzaczynski Oct 23, 2024

bzaczynski Oct 23, 2024

bzaczynski Oct 23, 2024

bzaczynski Oct 23, 2024

bzaczynski Oct 23, 2024

bzaczynski Oct 23, 2024

bzaczynski Oct 23, 2024

bzaczynski left a comment

bzaczynski left a comment

bzaczynski Mar 16, 2025

bzaczynski Mar 16, 2025

bzaczynski Mar 16, 2025

bzaczynski Mar 16, 2025


		## Run the Bandcamp Discover Player

		To run the music placer, navigate to the `src/` folder, then execute the module from your command-line:

	To run the music placer, navigate to the `src/` folder, then execute the module from your command-line:
	To run the music player, navigate to the `src/` folder, then execute the module from your command-line:

	"""Provides the main entry point for the app."""
	"""Provide the main entry point for the app."""


		The only direct dependency for this project is [Selenium](https://selenium-python.readthedocs.io/). You should use a Python version of at least 3.10, which is necessary to support [structural pattern matching](https://realpython.com/structural-pattern-matching/).

		You'll need a [Firefox Selenium driver](https://selenium-python.readthedocs.io/installation.html#drivers) called `geckodriver` to run the project as-is. Make sure to [download and install](https://github.com/mozilla/geckodriver/releases) it before running the project.

		return self._parent.find_element(*self.PLAY_BUTTON)


		class DiscoverTrackList(WebComponent, DiscoverPageLocator, TrackLocator):

Add slimmed-down Selenium project code #596

Are you sure you want to change the base?

Add slimmed-down Selenium project code #596

Conversation

martin-martin commented Oct 14, 2024

martin-martin commented Oct 14, 2024

Choose a reason for hiding this comment

bzaczynski Oct 23, 2024 • edited Loading

Choose a reason for hiding this comment

bzaczynski left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bzaczynski left a comment

Choose a reason for hiding this comment

bzaczynski left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bzaczynski Oct 23, 2024 •

edited

Loading