2
\$\begingroup\$

I am new to using Python and the following is the first script that I have written. I will execute a test complete project and will be started via team city. Once test execute has been called - the log file needs to be processed for consumption through TeamCity.

import sys
import argparse
import subprocess
import os.path

parser = argparse.ArgumentParser()
parser.add_argument("-s", "--projectSuite", help=".pjs that you want to execute", type=str)
parser.add_argument("-p", "--project", help=".pj that you want to execute", type=str, default="ExploratoryTests")
parser.add_argument("-l", "--logDirectory", help="The directory to log the mht's too", type=str, default="C:\TestLogs")
parser.add_argument("-lf", "--logFile", help="The name of the log", type=str, default="TextExecuteLog.mht")
parser.add_argument("-ps", "--psexec", help="Absolute path to psexec", type=str, default="c:\Program Files (x86)\PsTools\PsExec.exe")
parser.add_argument("-te", "--testExecute", help="Absolute path to testExecute", type=str, default="c:\Program Files (x86)\SmartBear\TestExecute 9\Bin\TestExecute.exe")
parser.add_argument("-m", "--mhtExtractor", help="Absolute path to an mht extractor", type=str, default="C:\extract2htm\extractMHT.exe")

#Get the args that are passed in
args = parser.parse_args()

#Make sure we have an absolute path
suite = os.path.abspath(args.projectSuite)

#Prepare the project for consumption by TestExecute
project = "/p:" + args.project

#Get a path for the currenct mht
logFile = os.path.abspath(os.path.join(args.logDirectory, args.logFile))

#Tell TestExecute where to put the log
log = "/exportlog:" + logFile

#prepare TE Executables
psexec = os.path.abspath(args.psexec);
testExecute = os.path.abspath(args.testExecute)

#Execute testExecute through psexec
testExecuteCommand = [psexec,"-i" ,"-accepteula", testExecute, suite, project, "/exit", log, "/r", "/DoNotShowLog", "/SilentMode"]

#Blank array to hold the TE outcome
testExecuteError = [];

try:
    result = subprocess.run(args = testExecuteCommand, stderr=subprocess.STDOUT, shell=True)
    result.check_returncode()
    testExecuteError.append(result)
except subprocess.CalledProcessError as e:
    result = e.returncode
    testExecuteError.append(e)
    print("Error code: {0}".format(result))

extractor = os.path.abspath(args.mhtExtractor)
outputDir = os.path.join(args.logDirectory)

extractorExecuteCommand = [extractor, logFile, outputDir]

try:
    result = subprocess.run(args = extractorExecuteCommand, stderr=subprocess.STDOUT, shell=True)
    result.check_returncode()
except subprocess.CalledProcessError as err:
    result = err.returncode
    print("Error code: {0}".format(result))


#exit the script with the exit code from testexetute
sys.exit(testExecuteError[0])

The way in which I handle my exceptions from testExecute are the part that I'm not happy about but it is working as intended. Any suggestions are welcome.

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

type=str is already the default way argparse interprets things. However, for all the options where the only further processing is calling os.path.abspath, you can use that as type. This affects args.psexec and args.testExecute.

Python has an official style-guide, PEP8, which you should follow. It recommends using lower_case_with_underscore for variables and functions.

Comments should explain why the code is the way it is, not what it does. The latter should be obvious from the code itself, the name of the function you encapsulated it in and/or the docstring accompanying that function.

In that vein, you could use more functions. One candidate for this would be get_args, where you can stick all your argparse code. Also the two subprocess calls could be in their own function, since they are completely independent. The least you should do is wrap your code in if __name__ == "__main__":. This way you can import this module from another script without running the tests.

In Python, ; serve (almost) no purpose, even though they are not a SyntaxError. You definitely do not need them at the end of normal lines.

Final code:

import sys
import argparse
import subprocess
import os.path

def get_args():
    parser = argparse.ArgumentParser()
    parser.add_argument("-s", "--projectSuite", help=".pjs that you want to execute", type=os.path.abspath)
    parser.add_argument("-p", "--project", help=".pj that you want to execute", default="ExploratoryTests")
    parser.add_argument("-l", "--logDirectory", help="The directory to log the mht's too", default="C:\TestLogs", type=os.path.join)
    parser.add_argument("-lf", "--logFile", help="The name of the log", default="TextExecuteLog.mht")
    parser.add_argument("-ps", "--psexec", help="Absolute path to psexec", type=os.path.abspath, default="c:\Program Files (x86)\PsTools\PsExec.exe")
    parser.add_argument("-te", "--testExecute", help="Absolute path to testExecute", type=os.path.abspath, default=r"C:\Program Files (x86)\SmartBear\TestExecute 9\Bin\TestExecute.exe")
    parser.add_argument("-m", "--mhtExtractor", help="Absolute path to an mht extractor", type=os.path.abspath, default="C:\extract2htm\extractMHT.exe")
    args = parser.parse_args()
    args.project = "/p:" + args.project
    args.logFile = os.path.abspath(os.path.join(args.logDirectory, args.logFile))
    args.log = "/exportlog:" + logFile
    return args


def execute_test(psexec, testExecute, suite, project, log, **kwargs):
    """Execute testExecute through psexec"""
    command = [psexec,"-i" ,"-accepteula", testExecute, suite, project, "/exit", log, "/r", "/DoNotShowLog", "/SilentMode"]

    #Blank array to hold the TE outcome
    testExecuteError = []
    try:
        result = subprocess.run(args=command, stderr=subprocess.STDOUT, shell=True)
        result.check_returncode()
        testExecuteError.append(result)
    except subprocess.CalledProcessError as e:
        testExecuteError.append(e)
        print("Error code: {0.returncode}".format(e))
    return testExecuteError


def execute_extractor(mhtExtractor, logDirectory, logFile, **kwargs):
    """Descriptive docstring"""
    command = [extractor, logFile, logDirectory]
    try:
        result = subprocess.run(args=command, stderr=subprocess.STDOUT, shell=True)
        result.check_returncode()
    except subprocess.CalledProcessError as e:
        print("Error code: {.returncode}".format(e))

if __name__ == "__main__":
    args = vars(get_args())
    testExecuteError = execute_test(**args)
    execute_extractor(**args)

    sys.exit(testExecuteError[0])

Note that I made all parameters part of the argparse.Namespace to have it all in one place. Afterwards I used vars to convert it to a dict. Then I used keyword arguments to make the function calls easier but at the same time giving the parameters of the function names.

To see how the dictionary unpacking works, have a look at this example:

def f(x, y, **kwargs):
    print x, y, kwargs

def g(x, z, **kwargs):
    print x, z, kwargs

args = {'x': 1, 'y': 2, 'z': 3}
f(**args)
# 1 2 {'z': 3}
f(1, 2)
# 1 2 {}
g(**args)
# 1 3 {'y': 2}

kwargs swallows all left-over keyword arguments and allows in my code above the passing in of the whole args dict, even though both functions need only some of its content.

\$\endgroup\$
1
  • \$\begingroup\$ Thank you kindly for showing me some tricks here - i have taken your comments on board and I will improve my script further. \$\endgroup\$
    – aydjay
    Jan 5, 2017 at 11:27

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.