How should I go about bringing this code under test?

Question

I'm working on an open-source test framework.

90% of my codebase has good test coverage. My main problem area is the command-line entry point. This module began life as a very short script for calling into the domain model (see the call to _run_impl()) and exit with code 1 if the test run failed.

It didn't feel necessary to test that script at the time that I wrote it, but as the project has sprouted features, a number of small changes to the logic in this file have led it to become quite long:

import argparse
import os
import sys
from . import _run_impl  # a function, in __init__.py, which calls into the domain model
from . import reporting  # a sub-package concerned with writing output to the console

import colorama; colorama.init()

def cmd():
    parser = argparse.ArgumentParser()
    parser.add_argument('-s', '--no-capture',
        action='store_false',
        dest='capture',
        default=True,
        help="Disable capturing of stdout during tests.")
    parser.add_argument('-v', '--verbose',
        action='store_true',
        dest='verbose',
        default=False,
        help="Enable verbose progress reporting.")
    parser.add_argument('--teamcity',
        action='store_true',
        dest='teamcity',
        default=False,
        help="Enable teamcity test reporting.")
    parser.add_argument('--no-random',
        action='store_false',
        dest='shuffle',
        default=True,
        help="Disable test order randomisation.")
    parser.add_argument('path',
        action='store',
        nargs='?',
        default=os.getcwd(),
        help="Path to the test file or directory to run. (default current directory)")
    args = parser.parse_args()

    if args.teamcity or "TEAMCITY_VERSION" in os.environ:
        reporters = (reporting.teamcity.TeamCityReporter(sys.stdout),)
    elif args.verbose:
        reporters = (reporting.cli.ColouredReporter(sys.stdout),)
    elif args.capture:
        reporters = (
            reporting.cli.DotsReporter(sys.stdout),
            type("ColouredCapturingReporter", (reporting.cli.ColouredReporter, reporting.cli.StdOutCapturingReporter), {})(sys.stdout),
            reporting.cli.TimedReporter(sys.stdout)
        )
    else:
        reporters = (
            reporting.cli.DotsReporter(sys.stdout),
            type("ColouredCapturingReporter", (reporting.cli.ColouredReporter, reporting.cli.SummarisingReporter), {})(sys.stdout),
            reporting.cli.TimedReporter(sys.stdout)
        )

    reporter = reporting.shared.ReporterManager(*reporters)

    _run_impl(os.path.realpath(args.path), reporter, args.shuffle)

    if reporter.failed:
        sys.exit(1)
    sys.exit(0)


if __name__ == "__main__":
    cmd()

I know that this code has bugs (for example, it should be possible to capture stdout from tests when running in verbose mode), and I've lost confidence in my ability to change this module without breaking something. So I want to write some tests for it - but I only really have experience with test-first development, so I don't really know where to start.

Should the tests be at the unit or integration level? Should I mock the ArgumentParser? Should I mock the reporting module? Should I mock _run_impl? Should I try to bring the whole module under test at the start, or just add tests as I add features? Do I need to refactor to make this method more testable?

Addendum: I think a part of my confusion is due to the fact that this is a relatively thin layer on top of the business logic - it's just there to make it possible to kick-off the test runner from the command line. So I'm feeling confused about what constitutes the service boundary here - in particular, what I should and shouldn't mock.

Answer 1

Testing is usually hard when several ideas are coupled together in the same code; it means testing a lot of things at the same time. Breaking code apart into smaller chunks usually solves that problem.

In that vein, I would rewrite this slightly by extracting some methods:

def cmd(arguments):
    args = parse(arguments)
    reporter = create_reporter(args)

    _run_impl(os.path.realpath(args.path), reporter, args.shuffle)

   return not reporter.failed

if __name__ == "__main__":
   if cmd(sys.argv)
       sys.exit(0)
   sys.exit(1)

This has several benefits:

• The code in cmd is not coupled to sys.argv and sys.exit(), which are both hard to deal with in a unit testing context
• cmd is now simple to test if you mock out parse, create_reporter, and _run_impl
• parse, create_reporter are simple and isolated enough to test easily on their own.

Python is not my primary language, so I can't recall if you'll need to create some classes to enable clean mocking, but if so do it. It's just a little bit of typing for a lot of value.

Answer 2

I think you should test the command-line interface as part of integration testing.

Change your main function to:

def cmd(args=None):
    ...
    args = parser.parse_args(args=args)
    ...

...so that you can easily pass in a list of arguments in your tests.

If running the entire program during testing is unreasonable, mock out _run_impl.

You might also want to look at something like scripttest to help capture output.

Answer 3

It seems like the main work is all outside this code. so tests for this code merely need to demonstrate that the calls it will perform are going to be performed reliably . Possibly you could also test that all of the 'reporters' lists will instantiate valid Reporter objects. This is all a perfectly legit case for plain old unit testing.

You can certainly break the code up into smaller bits suitable for unit testing -- for example, the argument parsing could become a function which would be fairly easy to test for different combinations of flags. It seems like the main job of this code is just to fill out the reporters list, so you should make sure that arguments produce the expected lists deterministically. Increasing testability might also suggest a refactor in which "reporters" becomes a class rather than a list so you can understand it's contents in a structured way -- that will be more testable and also perhaps more maintainable down the road, though it's hard to tell without knowing the APIs

Overall though this seems like a fairly testable scenario, and (like a lot of UI code ) one which would benefit from a battery of tests that make sure things like poorly formed or inconsistent arguments are handled in a reliable way.