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 <[email protected]>

…ials into python-selenium

bzaczynski

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

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 <[email protected]>

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

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


		## 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."""

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 Oct 23, 2024 •

edited

Loading