I have started a hobby project and would like to integrate better and more thorough testing practices. Since I am not too used to it, I'd love to have some feedback on the sample code below. In particular, I'd be interested in the best practices surrounding setting up data necessary to feed a particular testing function or set of functions. As it probably matters, I intend to use pytest
.
Here is the sample code from the first testing module in my repo so far:
from accounts import Account
from accounts import AccountRollup
from accounts import AccountType
from accounts import AmountType
from accounts import JournalEntry
from accounts import JournalEntryLeg
# Setup chart of accounts
cash_account = Account(1, "Cash", AccountType.Asset)
mortgage_account = Account(2, "Mortgage", AccountType.Liability)
revenue_account = Account(3, "Revenues", AccountType.Income)
balance_sheet = AccountRollup([cash_account, mortgage_account])
income_statement = AccountRollup(revenue_account)
chart_of_accounts = AccountRollup([balance_sheet, income_statement])
def test_journal_entry_balance():
# Journal entry is balanced if Debit == Credit
jnl = JournalEntry(
[
JournalEntryLeg(cash_account, 50, AmountType.Debit),
JournalEntryLeg(cash_account, 50, AmountType.Debit),
JournalEntryLeg(mortgage_account, 100, AmountType.Credit),
]
)
assert jnl.is_balanced()
# Journal entry is not balanced if Debit != Credit
jnl = JournalEntry(
[
JournalEntryLeg(cash_account, 100, AmountType.Debit),
JournalEntryLeg(mortgage_account, 50, AmountType.Credit),
]
)
assert not jnl.is_balanced()
# Journal entry is not balanced even if posting negative amounts
jnl = JournalEntry(
[
JournalEntryLeg(cash_account, 100, AmountType.Debit),
JournalEntryLeg(mortgage_account, -100, AmountType.Debit),
]
)
assert not jnl.is_balanced()
# Test floating-point precision (This test should fail, just testing pytest)
jnl = JournalEntry(
[
JournalEntryLeg(cash_account, 100, AmountType.Debit),
JournalEntryLeg(mortgage_account, 99.9, AmountType.Credit),
]
)
assert jnl.is_balanced()
If I had to guess areas of improvement:
- I find the code very repetitive and verbose. It could probably be worthwhile grouping variable assignments and assertions together instead of separated by test case.
- Set variable assignments in global variables doesn't look very too clean.
2 Answers 2
The first thing I would point out is that currently, you have multiple test cases (or multiple asserts) inside a single test function. A major downside of this is that if any of the asserts fail, the test function will exit immediately and not run the remaining test cases, making locating and debugging failing tests all that bit harder.
There are a couple of approaches to fix this:
Have a separate test function for each, with the name of the function describing precisely what the test does. I typically follow the format of
test_<subject>_with_<something>_check_<condition>
. For example, you could split your current code into 4 separate test functions named:test_balance_with_debit_equal_to_credit_check_balanced
test_balance_with_debit_greater_than_credit_check_not_balanced
test_balance_with_negative_balance_amounts_check_not_balanced
test_balance_with_small_float_difference_check_not_balanced
Use pytest's parameterize functionality to create sub-tests for us. I personally wouldn't use this approach for the code you have provided, I favour the more explicit result of #1 until we had many more test cases to consider.
As for the global variables, for the size of this, I don't think it's too bad but you might find pytest's fixtures concept useful in the future. These are useful for employing an Arrange-Act-Assert (AAA) pattern of testing[1][2] and allow you to extract common data or setup to be reused easily.
@pytest.fixture
def cash_account():
return Account(1, "Cash", AccountType.Asset)
@pytest.fixture
def mortgage_account():
return Account(2, "Mortgage", AccountType.Asset)
@pytest.fixture
def revenue_account():
return Account(3, "Revenues", AccountType.Asset)
# name the fixtures you would like to use as parameters to the test
def test_balance_with_debit_equal_to_credit_check_balanced(cash_account, mortgage_account):
journal = JournalEntry(
[
JournalEntryLeg(cash_account, 50, AmountType.Debit),
JournalEntryLeg(cash_account, 50, AmountType.Debit),
JournalEntryLeg(mortgage_account, 100, AmountType.Credit),
]
)
assert jnl.is_balanced()
You can also build fixtures from other fixtures like so:
@pytest.fixture
def account_charter(cash_account, mortgage_account, revenue_account):
balance_sheet = AccountRollup([cash_account, mortgage_account])
income_statement = AccountRollup(revenue_account)
return AccountRollup([balance_sheet, income_statement])
The chart_of_accounts
variable is unused in the example you provided so I'm not sure if this particular example is relevant but hopefully, you will find a use for the technique elsewhere.
Useful Reading
Pytest covers a bit about AAA in its fixtures documentation I linked above but I also very much liked these blog posts on the topic:
Definitely not the most important issue, but at least for my brain, this is a case where some short convenience variables -- with contextual help and a narrow scope -- can really help code readability scannability. Just has a nicer vibe too.
def get_journal_entry(*tups):
return JournalEntry([JournalEntryLeg(*t) for t in tups])
def test_journal_entry_balance():
ca = cash_account
mo = mortgage_account
d = AmountType.Debit
c = AmountType.Credit
# Journal entry is balanced if Debit == Credit
jnl = get_journal_entry(
(ca, 50, d),
(ca, 50, d),
(mo, 100, c),
)
assert jnl.is_balanced()
# Journal entry is not balanced if Debit != Credit
jnl = JournalEntry(
(ca, 100, d),
(mo, 50, d),
)
assert not jnl.is_balanced()
# Journal entry is not balanced even if posting negative amounts
jnl = JournalEntry(
(ca, 100, d),
(mo, -100, d),
)
assert not jnl.is_balanced()
# Test floating-point precision (This test should fail, just testing pytest)
jnl = JournalEntry(
(ca, 100, d),
(mo, 99.9, c),
)
assert jnl.is_balanced()