Run several Python programs and check for errors

Question

For a post-graduate subject that I present, I maintain a repository of examples (on GitHub) from the textbook which is contributed and edited by my students, who are not very Python literate. I have written this code to run all the programs associated with figures, examples and exercises in the book and use it to alert me if a pull request has broken anything via Travis. Each program is a simple Python script which does not use any special stuff to do what it does. I don't check for correctness, just if the program actually runs. Many of the programs produce figures, so I've had to do some dirty things with matplotlib's show command so that these programs don't halt execution. This works but it all feels very hacky to me, and I'd be interested in suggestions to improve this checking.

You can check the repo for all the example files, but the main running script is here:

#!/usr/bin/env python

from __future__ import print_function
from past.builtins import execfile
import os
import traceback
import re
from collections import Counter
import sys

# disable show in figures
import matplotlib.pyplot as plt
plt.show = lambda: None

statuscounter = Counter()

itemparser = re.compile('(?P<kind>.*) (?P<chapter>.*)\.(?P<number>.*)')
allitems = open('allitems.txt').read().splitlines()

kinds = ['Figure', 'Example', 'Exercise']

faillist = []

if __name__ == "__main__":
    for item in allitems:
        kind, chapter_c, number_c = itemparser.match(item).groups()

        number = int(number_c)

        if chapter_c.isdigit():
            chapter = int(chapter_c)
            mask = '{}_{:02d}_{:02d}.py'
        else:
            chapter = chapter_c
            mask = '{}_{}_{}.py'

        filename = mask.format(kind, chapter, number)
        if os.path.exists(filename):
            try:
                execfile(filename)
                status = 'Success'
            except Exception as err:
                status = 'Failed'
                message = traceback.format_exc()
        else:
            status = 'Not implemented'

        statuscounter[status] += 1

        if status != 'Not implemented':
            print(kind, chapter, number, status)

        if status == 'Failed':
            faillist.append([kind, chapter, number])
            print(message)

    for items in statuscounter.items():
        print("{}: {}".format(*items))
    print("Failed items:")
    for items in faillist:
        print("  {} {} {}".format(*items))

    sys.exit(statuscounter['Failed'])

Answer 1

allitems = open('allitems.txt').read().splitlines()

You're putting all of those lines into a list, but you don't need them as a list. All that you do with them later is iterate, so you should just say allitems = open('allitems.txt'). That will mean that the newline characters will be at the end of each line, but you can fix that by adding item = item.strip() in your loop. You should also always use a with statement when opening files.

kind, chapter_c, number_c = itemparser.match(item).groups()

What if the item doesn't match? You'll have a very ugly error that won't make sense to whoever uses your program. You should have some error handling added:

item = item.strip()
match = itemparser.match(item):
if not match or len(match.groups()) != 3:
    print("Invalid item: {}".format(item), file=sys.stderr)
    continue

If you want, you could throw an error instead, but your error would hopefully be more helpful to the user.

number = int(number_c)

Again, no error handling.

status = 'Success'
...
status = 'Failed'
...
status = 'Not implemented'

Those are hard-coded values. What would happen if you had just one slight typo? It could throw you off with something completely unexpected and it would be very difficult to debug. You should define constants at the beginning of the file for the different status codes:

SUCCESS = 'Success'
FAILED = 'Failed'

Well, you don't really need to define one for 'Not implemented' because NotImplemented already exists as a built-in constant. You could even use True, False, and NotImplemented. In that case, the variable might be success instead of status.

sys.exit(statuscounter['Failed'])

I like that. You exit with success if nothing failed, but the error exit codes make sense.

Answer 2

• Ask forgiveness, not permission. The test

  if os.path.exists(filename):
  

  is subject to a TOCTOU race condition, and is generally non-pythonic. Consider instead

  try:
      execfile(...)
  except IOError as e:
      # No such file or directory
  

• I hope the execution environment is chrooted, permissions are set to absolute minimum, etc.