-
Notifications
You must be signed in to change notification settings - Fork 0
TestCodingStyle
These are quick notes to help you fix common problems autotest/virt-test code submissions usually have. Please read this and keep it in mind when writing code for these projects:
While we understand that sometimes the contributions in question are adaptations of existing shell scripts, we ask you to avoid needlessly use shell script constructs that can be easily replaced by standard python API. Common cases:
- Use of rm, when you can use os.remove(), and rm -rf when you can use shutil.rmtree.
Please don't
os.system('rm /tmp/foo')
Do
os.remove('/tmp/foo')
Please don't
os.system('rm -rf /tmp/foo')
Do
shutil.rmtree('/tmp/foo')
- Use of cat when you want to write contents to a file
Please, really, don't
cmd = """cat << EOF > %s Hey, this is a multiline text to % EOF""" % (some_file, some_string) commands.getstatusoutput(cmd)
Do
content = """ Hey, this is a multiline text to %s """ % some_string some_file_obj = open(some_file, 'w') some_file_obj.write(content) some_file_obj.close()
Autotest already provides utility methods that are preferrable over os.system or commands.getstatus() and the likes. The APIs are called utils.system, utils.run, utils.system_output. They raise exceptions in case of a return code !=0, so keep this in mind (either you pass ignore_status=True or trap an exception in case you want something different other than letting this exception coalesce and fail your test).
from autotest.client.shared import error from autotest.client import utils # Raises exception, use with error.context error.context('Disabling firewall') utils.system('iptables -F') # If you just want the output output = utils.system_output('dmidecode') # Gives a cmdresult object, that has .stdout, .stderr attributes cmdresult = utils.run('lspci') if not "MYDEVICE" in cmdresult.stdout(): raise error.TestError("Special device not found")
In general the use of backslashes is really ugly, and it can be avoided pretty much every time. Please don't use
long_cmd = "foo & bar | foo & bar | foo & bar | foo & bar | foo & bar \ foo & bar"
instead, use
long_cmd = ("foo & bar | foo & bar | foo & bar | foo & bar | foo & bar " "foo & bar")
So, parentheses can avoid the use of backslashes in long lines and commands.
Autotest projects use strictly python 2.4, so you can't use constructs that appeared in newer versions of python, some examples:
try: foo() except BarError as details: # except ExceptionClass as variable was introduced after 2.4 baz
try: foo() except BarError, details: # correct, 2.4 compliant syntax baz() finally: # This is the problem, try/except/finally blocks were introduced after 2.4 gnu()
So, when in doubt, consult the python documentation before sending the patch.