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()
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Jun 11, 2017 at 0:12
\$\endgroup\$

1 Answer 1

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.

answered Jun 11, 2017 at 1:08
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.