2
\$\begingroup\$

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.
\$\endgroup\$
2
\$\begingroup\$

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:

  1. 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
  2. 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:

[1] - AAA Testing Pattern: Part 1

[2] - AAA Testing Pattern: Part 2

\$\endgroup\$
1
\$\begingroup\$

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()
\$\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.