3
\$\begingroup\$

This is part of a program I'm working on to help me manage an event in a subreddit I'm involved with. The snippet here parses a post and returns the relevant information.

I think I'm most interested in feedback on how I've set up the unit tests. Also, if there's any cleaner way to extract the information.

Handling invalid input is something I have not started yet.

src/pianojam.py

import configparser
import re

import praw
import praw.models

DELIMITER = '---'  # type: str
CATEGORIES = ['Jazz', 'Classical', 'Ragtime', 'Video Game / Anime / Film']  # type: [str]


def init_reddit(config_pathname: str) -> praw.Reddit:
    config = configparser.ConfigParser()
    config.read(config_pathname)
    return praw.Reddit(client_id=config['RedditParams']['client_id'],
                       client_secret=config['RedditParams']['client_secret'],
                       user_agent=config['RedditParams']['user_agent'])


REDDIT = init_reddit('config.ini')


class Piece(object):
    """A piece to be listed in the piano jam"""

    def __init__(self, piece_text: str):
        # Expected format: Composer: [Title](url) | [Sheet Music](sheetUrl)
        self.composer = piece_text[:piece_text.index(':')]  # type: str
        self.title = re.findall(re.compile('\[(.*?)\]'), piece_text)[0]  # type: str
        urls = re.findall(re.compile('\((.*?)\)'), piece_text)
        self.video_url = urls[0]  # type: str
        self.score_url = urls[1]  # type: str


def format_title(category: str) -> str:
    return '**{}**'.format(category)


def parse_pieces(section_text: str) -> [Piece]:
    pieces = section_text.split('\n')[1:]  # First line is the category; discard
    return (Piece(piece_text) for piece_text in pieces if piece_text.strip() != '')

def get_selections_from_jam(jam_text: str) -> [Piece]:
    sections = jam_text.split(DELIMITER)
    sections = (section.strip() for section in sections)
    filtered_sections = []
    for section in sections:
        section = section.strip()
        for category in CATEGORIES:
            category = format_title(category)
            if section.startswith(category):
                filtered_sections.append(section)
                break
    pieces = []
    for section in filtered_sections:
        pieces.extend(parse_pieces(section))
    return pieces


def get_selections_from_url(url: str) -> [Piece]:
    post = praw.models.Submission(REDDIT, url=url)
    return get_selections_from_jam(post.selftext)

tests/test_parsing.py

import unittest

import pianojam
from pianojam import Piece


class TestParsingJam(unittest.TestCase):
    @classmethod
    def setUpClass(cls):
        cls.jam_text = ("\n"
                        "**Submissions from last month's Piano Jam**\n"
                        "\n"
                        "[Submission information]\n"
                        "---\n"
                        "\n"
                        "**Guidelines:**\n"
                        "\n"
                        "[Guidelines]\n"
                        "---\n"
                        "\n"
                        "[Composition]\n"
                        "\n"
                        "---\n"
                        "\n"
                        "**Classical**\n"
                        "\n"
                        "Classical Composer: [Classical Title](url) | [Sheet Music](sheetUrl)\n"
                        "Second Composer: [Another Title](nextPieceUrl) | [Sheet Music](secondSheetUrl)\n"
                        "\n"
                        "---\n"
                        "\n"
                        "**Jazz**\n"
                        "\n"
                        "Jazz Composer: [Jazz Title](jazzUrl) | [Lead Sheet](leadUrl)\n"
                        "\n"
                        "---\n"
                        "\n"
                        "---\n"
                        "\n"
                        "Have fun!")
    def testGrabsAllSelections(self):
        selections = pianojam.get_selections_from_jam(self.jam_text)
        self.assertEqual(len(selections), 3)

    def testSelectionPopulatedCorrectly(self):
        selections = pianojam.get_selections_from_jam(self.jam_text)
        jazz_piece = None  # type: Piece
        for selection in selections:
            if selection.title == "Jazz Title":
                jazz_piece = selection
                break

        self.assertIsNotNone(jazz_piece, 'No piece with title "Jazz Piece"')
        self.assertEqual(jazz_piece.composer, 'Jazz Composer')
        self.assertEqual(jazz_piece.video_url, 'jazzUrl')
        self.assertEqual(jazz_piece.score_url, 'leadUrl')


if __name__ == '__main__':
    unittest.main()
\$\endgroup\$
2
\$\begingroup\$

I would move your jam_text text into a separate file or a multiline constant string to keep the test itself cleaner.

Also, this block:

jazz_piece = None  # type: Piece
for selection in selections:
    if selection.title == "Jazz Title":
        jazz_piece = selection
        break

can be simplified via using next():

jazz_piece = next((selection for selection in selections if selection.title == "Jazz Title"), None)

Note about naming - even though the class name and the setUpClass should be in camel case and they are, your test methods should not be - use lower_case_with_underscores naming style for them.

Aside from that, since this code is going grow, look into parameterizing your tests with subtests, or using ddt package, or using pytest fixtures.

Measure code coverage and make sure you cover not only the "happy" paths, but the error-handling paths as well.

And, I think you can even apply some Property-based Testing - check out hypothesis library - I cannot stress enough how much did it help me in unit-testing and catching the bugs I was not even thinking about.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.