Submission common problems¶
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:
Gratuitous use of shell script inside a python program¶
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.
os.system('rm -rf /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)
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()
Use of the commands API, or os.system¶
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")
Use of backslashes¶
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"
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.
Use of constructs that appeared in versions of python > 2.4¶
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.
Unconditional import of external python libraries¶
Sometimes, for a tiny feature inside the test suite, people import an external, lesser known python library, on a very central and proeminent part of the framework.
Please, don’t do it. You are breaking other people’s workflow and that is bad.
The correct way of doing this is conditionally importing the library, setting a top level variable that indicates whether the feature is active in the system (that is, the library can be imported), and when calling the specific feature, check the top level variable to see if the feature could be found. If it couldn’t, you fail the test, most probably by throwing an autotest.client.shared.error.TestNAError.
So, instead of doing:
import platinumlib ... platinumlib.destroy_all()
You will do:
PLATINUMLIB_ENABLED = True try: import platinumlib except ImportError: PLATINUMLIB_ENABLED = False ... if not PLATINUMLIB_ENABLED: raise error.TestNAError('Platinum lib is not installed. ' 'You need to install the package ' 'python-platinumlib for this test ' 'to work.') platinumlib.destroy_all()
Any patch that carelessly sticks external library imports in central libraries of virt-test for optional features will be downright rejected.