Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 1 | # Coding style for autotest in Chrome OS / Android / Brillo |
Christopher Wiley | 2f011d9 | 2016-02-04 09:35:32 -0800 | [diff] [blame] | 2 | These rules elaborate on, but rarely deviate from PEP-8. When in doubt, go |
| 3 | with PEP-8. |
mbligh | 71d338a | 2007-10-08 05:05:50 +0000 | [diff] [blame] | 4 | |
| 5 | |
Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 6 | ## Language |
Christopher Wiley | 2f011d9 | 2016-02-04 09:35:32 -0800 | [diff] [blame] | 7 | * Use Python where possible |
| 8 | * Prefer writing more Python to a smaller amount of shell script in host |
| 9 | commands. In practice, the Python tends to be easier to maintain. |
| 10 | * Some tests use C/C++ in test dependencies, and this is also ok. |
mbligh | 2ac475b | 2008-09-09 21:37:40 +0000 | [diff] [blame] | 11 | |
| 12 | |
Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 13 | ## Indentation & whitespace |
mbligh | 71d338a | 2007-10-08 05:05:50 +0000 | [diff] [blame] | 14 | |
| 15 | Format your code for an 80 character wide screen. |
| 16 | |
Christopher Wiley | 2f011d9 | 2016-02-04 09:35:32 -0800 | [diff] [blame] | 17 | Indentation is 4 spaces, as opposed to hard tabs (which it used to be). |
mbligh | c960fcf | 2008-06-18 19:58:57 +0000 | [diff] [blame] | 18 | This is the Python standard. |
mbligh | 71d338a | 2007-10-08 05:05:50 +0000 | [diff] [blame] | 19 | |
Tan Gao | 8aac17b | 2013-04-16 08:46:01 -0700 | [diff] [blame] | 20 | For hanging indentation, use 8 spaces plus all args should be on the new line. |
| 21 | |
Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 22 | ``` |
Christopher Wiley | 2f011d9 | 2016-02-04 09:35:32 -0800 | [diff] [blame] | 23 | |
Tan Gao | 8aac17b | 2013-04-16 08:46:01 -0700 | [diff] [blame] | 24 | # Either of the following hanging indentation is considered acceptable. |
| 25 | YES: return 'class: %s, host: %s, args = %s' % ( |
| 26 | self.__class__.__name__, self.hostname, self.args) |
| 27 | |
| 28 | YES: return 'class: %s, host: %s, args = %s' % ( |
| 29 | self.__class__.__name__, |
| 30 | self.hostname, |
| 31 | self.args) |
| 32 | |
| 33 | # Do not use 4 spaces for hanging indentation |
| 34 | NO: return 'class: %s, host: %s, args = %s' % ( |
| 35 | self.__class__.__name__, self.hostname, self.args) |
| 36 | |
| 37 | # Do put all args on new line |
| 38 | NO: return 'class: %s, host: %s, args = %s' % (self.__class__.__name__, |
| 39 | self.hostname, self.args) |
Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 40 | ``` |
Tan Gao | 8aac17b | 2013-04-16 08:46:01 -0700 | [diff] [blame] | 41 | |
mbligh | 71d338a | 2007-10-08 05:05:50 +0000 | [diff] [blame] | 42 | Don't leave trailing whitespace, or put whitespace on blank lines. |
Simran Basi | c7330bd | 2013-05-31 09:23:50 -0700 | [diff] [blame] | 43 | |
mbligh | 71d338a | 2007-10-08 05:05:50 +0000 | [diff] [blame] | 44 | |
Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 45 | ## Variable names and UpPeR cAsE |
Christopher Wiley | 2f011d9 | 2016-02-04 09:35:32 -0800 | [diff] [blame] | 46 | * Use descriptive variable names where possible |
| 47 | * Use `variable_names_like_this` |
| 48 | * Use `method_and_function_names_like_this` |
| 49 | * Use `UpperCamelCase` for class names |
mbligh | 71d338a | 2007-10-08 05:05:50 +0000 | [diff] [blame] | 50 | |
mbligh | d876f45 | 2008-12-03 15:09:17 +0000 | [diff] [blame] | 51 | |
Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 52 | ## Importing modules |
mbligh | 7654c82 | 2008-04-04 15:12:48 +0000 | [diff] [blame] | 53 | |
| 54 | The order of imports should be as follows: |
| 55 | |
Christopher Wiley | 2f011d9 | 2016-02-04 09:35:32 -0800 | [diff] [blame] | 56 | * Standard python modules |
| 57 | * Non-standard python modules |
| 58 | * Autotest modules |
mbligh | 7654c82 | 2008-04-04 15:12:48 +0000 | [diff] [blame] | 59 | |
| 60 | Within one of these three sections, all module imports using the from |
| 61 | keyword should appear after regular imports. |
Christopher Wiley | d7445ef | 2014-12-05 13:26:05 -0800 | [diff] [blame] | 62 | Each module should be imported on its own line. |
Christopher Wiley | 2f011d9 | 2016-02-04 09:35:32 -0800 | [diff] [blame] | 63 | Do not use Wildcard imports (`from x import *`) if possible. |
| 64 | |
| 65 | Import modules, not classes. For example: |
Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 66 | |
| 67 | ``` |
mbligh | 7654c82 | 2008-04-04 15:12:48 +0000 | [diff] [blame] | 68 | from common_lib import error |
Christopher Wiley | 2f011d9 | 2016-02-04 09:35:32 -0800 | [diff] [blame] | 69 | |
| 70 | def foo(): |
| 71 | raise error.AutoservError(...) |
Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 72 | ``` |
| 73 | |
| 74 | and not: |
| 75 | |
| 76 | ``` |
mbligh | 7654c82 | 2008-04-04 15:12:48 +0000 | [diff] [blame] | 77 | from common_lib.error import AutoservError |
Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 78 | ``` |
mbligh | 7654c82 | 2008-04-04 15:12:48 +0000 | [diff] [blame] | 79 | |
| 80 | For example: |
Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 81 | |
| 82 | ``` |
Christopher Wiley | d7445ef | 2014-12-05 13:26:05 -0800 | [diff] [blame] | 83 | import os |
| 84 | import pickle |
| 85 | import random |
| 86 | import re |
| 87 | import select |
| 88 | import shutil |
| 89 | import signal |
| 90 | import subprocess |
| 91 | |
mbligh | 8bcd23a | 2009-02-03 19:14:06 +0000 | [diff] [blame] | 92 | import common # Magic autotest_lib module and sys.path setup code. |
| 93 | import MySQLdb # After common so that we check our site-packages first. |
Christopher Wiley | d7445ef | 2014-12-05 13:26:05 -0800 | [diff] [blame] | 94 | |
mbligh | 7654c82 | 2008-04-04 15:12:48 +0000 | [diff] [blame] | 95 | from common_lib import error |
Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 96 | ``` |
mbligh | 7654c82 | 2008-04-04 15:12:48 +0000 | [diff] [blame] | 97 | |
Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 98 | ## Testing None |
mbligh | d876f45 | 2008-12-03 15:09:17 +0000 | [diff] [blame] | 99 | |
Christopher Wiley | 2f011d9 | 2016-02-04 09:35:32 -0800 | [diff] [blame] | 100 | Use `is None` rather than `== None` and `is not None` rather than `!= None`. |
Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 101 | This way you'll never run into a case where someone's `__eq__` or `__ne__` |
Christopher Wiley | 2f011d9 | 2016-02-04 09:35:32 -0800 | [diff] [blame] | 102 | method does the wrong thing. |
mbligh | d876f45 | 2008-12-03 15:09:17 +0000 | [diff] [blame] | 103 | |
mbligh | 71d338a | 2007-10-08 05:05:50 +0000 | [diff] [blame] | 104 | |
Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 105 | ## Comments |
mbligh | 71d338a | 2007-10-08 05:05:50 +0000 | [diff] [blame] | 106 | |
| 107 | Generally, you want your comments to tell WHAT your code does, not HOW. |
| 108 | We can figure out how from the code itself (or if not, your code needs fixing). |
| 109 | |
| 110 | Try to describle the intent of a function and what it does in a triple-quoted |
| 111 | (multiline) string just after the def line. We've tried to do that in most |
mbligh | 5220736 | 2009-09-03 20:54:29 +0000 | [diff] [blame] | 112 | places, though undoubtedly we're not perfect. A high level overview is |
mbligh | 71d338a | 2007-10-08 05:05:50 +0000 | [diff] [blame] | 113 | incredibly helpful in understanding code. |
| 114 | |
| 115 | |
Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 116 | ## Hardcoded String Formatting |
Simran Basi | c7330bd | 2013-05-31 09:23:50 -0700 | [diff] [blame] | 117 | |
Dana Goyette | 9cf68d1 | 2019-07-16 10:30:18 -0700 | [diff] [blame] | 118 | Strings should use only single quotes for hardcoded strings in the code. |
| 119 | Double quotes are acceptable when single quote is used as part of the string. |
Christopher Wiley | 2f011d9 | 2016-02-04 09:35:32 -0800 | [diff] [blame] | 120 | Multiline strings should not use '\' but wrap the string using parentheseses. |
Simran Basi | c7330bd | 2013-05-31 09:23:50 -0700 | [diff] [blame] | 121 | |
Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 122 | ``` |
Simran Basi | c7330bd | 2013-05-31 09:23:50 -0700 | [diff] [blame] | 123 | REALLY_LONG_STRING = ('This is supposed to be a really long string that is ' |
| 124 | 'over 80 characters and does not use a slash to ' |
| 125 | 'continue.') |
Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 126 | ``` |
Simran Basi | c7330bd | 2013-05-31 09:23:50 -0700 | [diff] [blame] | 127 | |
Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 128 | ## Docstrings |
mbligh | 5cad50f | 2009-06-08 16:50:51 +0000 | [diff] [blame] | 129 | |
| 130 | Docstrings are important to keep our code self documenting. While it's not |
| 131 | necessary to overdo documentation, we ask you to be sensible and document any |
| 132 | nontrivial function. When creating docstrings, please add a newline at the |
showard | 25f056a | 2009-11-23 20:22:50 +0000 | [diff] [blame] | 133 | beginning of your triple quoted string and another newline at the end of it. If |
| 134 | the docstring has multiple lines, please include a short summary line followed |
| 135 | by a blank line before continuing with the rest of the description. Please |
| 136 | capitalize and punctuate accordingly the sentences. If the description has |
| 137 | multiple lines, put two levels of indentation before proceeding with text. An |
| 138 | example docstring: |
mbligh | 5cad50f | 2009-06-08 16:50:51 +0000 | [diff] [blame] | 139 | |
Dana Goyette | 9cf68d1 | 2019-07-16 10:30:18 -0700 | [diff] [blame] | 140 | ```python |
mbligh | 5cad50f | 2009-06-08 16:50:51 +0000 | [diff] [blame] | 141 | def foo(param1, param2): |
mbligh | 5220736 | 2009-09-03 20:54:29 +0000 | [diff] [blame] | 142 | """ |
showard | 25f056a | 2009-11-23 20:22:50 +0000 | [diff] [blame] | 143 | Summary line. |
| 144 | |
mbligh | 5220736 | 2009-09-03 20:54:29 +0000 | [diff] [blame] | 145 | Long description of method foo. |
mbligh | 5cad50f | 2009-06-08 16:50:51 +0000 | [diff] [blame] | 146 | |
Dana Goyette | 9cf68d1 | 2019-07-16 10:30:18 -0700 | [diff] [blame] | 147 | @param param1: A Spam object that has methods bar() and baz() |
| 148 | which raise SpamError if something goes awry. |
| 149 | @type param1: Spam |
Simran Basi | c7330bd | 2013-05-31 09:23:50 -0700 | [diff] [blame] | 150 | |
Dana Goyette | 9cf68d1 | 2019-07-16 10:30:18 -0700 | [diff] [blame] | 151 | @return: a list of integers describing changes in a source tree |
| 152 | @rtype: list |
Simran Basi | c7330bd | 2013-05-31 09:23:50 -0700 | [diff] [blame] | 153 | |
Dana Goyette | 9cf68d1 | 2019-07-16 10:30:18 -0700 | [diff] [blame] | 154 | @raise SpamError: when some stated condition occurs. |
Simran Basi | c7330bd | 2013-05-31 09:23:50 -0700 | [diff] [blame] | 155 | |
mbligh | 5220736 | 2009-09-03 20:54:29 +0000 | [diff] [blame] | 156 | """ |
Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 157 | ``` |
mbligh | 5cad50f | 2009-06-08 16:50:51 +0000 | [diff] [blame] | 158 | |
| 159 | The tags that you can put inside your docstring are tags recognized by systems |
Dana Goyette | 9cf68d1 | 2019-07-16 10:30:18 -0700 | [diff] [blame] | 160 | like epydoc. Not all places need all tags defined, so choose them wisely while |
| 161 | writing code. Generally always list any parameters, the return value (if there |
| 162 | is one), and exceptions that can be raised. |
mbligh | 5cad50f | 2009-06-08 16:50:51 +0000 | [diff] [blame] | 163 | |
Dana Goyette | 9cf68d1 | 2019-07-16 10:30:18 -0700 | [diff] [blame] | 164 | | Tag | Description |
| 165 | |----------|------------------------------------------------------------------- |
| 166 | | @author | Code author |
| 167 | | @cvar | Class variable (defined on `self.__class__.`) |
| 168 | | @ivar | Instance variable for a class (defined on `self.`) |
| 169 | | @param | Parameter description |
| 170 | | @raise | Exception type the function can throw, and the conditions for it |
| 171 | | @return | Return value description |
| 172 | | @rtype | The type the method returns |
| 173 | | @see | Reference to other parts of the codebase |
| 174 | | @type | The type of a given parameter or variable |
| 175 | | @warning | Call attention to potential problems with the code |
| 176 | | @version | Version string |
mbligh | 5cad50f | 2009-06-08 16:50:51 +0000 | [diff] [blame] | 177 | |
Dana Goyette | 9cf68d1 | 2019-07-16 10:30:18 -0700 | [diff] [blame] | 178 | For additional information and fields, refer to the epydoc manual: |
| 179 | http://epydoc.sourceforge.net/manual-fields.html |
mbligh | 3bdba92 | 2010-05-03 18:02:54 +0000 | [diff] [blame] | 180 | |
Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 181 | ## Simple code |
mbligh | 71d338a | 2007-10-08 05:05:50 +0000 | [diff] [blame] | 182 | |
| 183 | Keep it simple; this is not the right place to show how smart you are. We |
| 184 | have plenty of system failures to deal with without having to spend ages |
| 185 | figuring out your code, thanks ;-) Readbility, readability, readability. |
Christopher Wiley | 2f011d9 | 2016-02-04 09:35:32 -0800 | [diff] [blame] | 186 | Strongly prefer readability and simplicity over compactness. |
mbligh | 71d338a | 2007-10-08 05:05:50 +0000 | [diff] [blame] | 187 | |
| 188 | "Debugging is twice as hard as writing the code in the first place. Therefore, |
mbligh | 5220736 | 2009-09-03 20:54:29 +0000 | [diff] [blame] | 189 | if you write the code as cleverly as possible, you are, by definition, not |
mbligh | 71d338a | 2007-10-08 05:05:50 +0000 | [diff] [blame] | 190 | smart enough to debug it." Brian Kernighan |
| 191 | |
| 192 | |
Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 193 | ## Function length |
mbligh | 71d338a | 2007-10-08 05:05:50 +0000 | [diff] [blame] | 194 | |
| 195 | Please keep functions short, under 30 lines or so if possible. Even though |
Christopher Wiley | 2f011d9 | 2016-02-04 09:35:32 -0800 | [diff] [blame] | 196 | you are amazingly clever, and can cope with it, the rest of us are busy and |
| 197 | stupid, so be nice and help us out. To quote the Linux Kernel coding style: |
mbligh | 71d338a | 2007-10-08 05:05:50 +0000 | [diff] [blame] | 198 | |
| 199 | Functions should be short and sweet, and do just one thing. They should |
| 200 | fit on one or two screenfuls of text (the ISO/ANSI screen size is 80x24, |
| 201 | as we all know), and do one thing and do that well. |
| 202 | |
| 203 | |
Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 204 | ## Exceptions |
mbligh | 900b6c1 | 2008-01-14 16:56:47 +0000 | [diff] [blame] | 205 | |
| 206 | When raising exceptions, the preferred syntax for it is: |
| 207 | |
Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 208 | ``` |
mbligh | 900b6c1 | 2008-01-14 16:56:47 +0000 | [diff] [blame] | 209 | raise FooException('Exception Message') |
Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 210 | ``` |
mbligh | 900b6c1 | 2008-01-14 16:56:47 +0000 | [diff] [blame] | 211 | |
| 212 | Please don't raise string exceptions, as they're deprecated and will be removed |
| 213 | from future versions of python. If you're in doubt about what type of exception |
| 214 | you will raise, please look at http://docs.python.org/lib/module-exceptions.html |
Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 215 | and client/common\_lib/error.py, the former is a list of python built in |
mbligh | 900b6c1 | 2008-01-14 16:56:47 +0000 | [diff] [blame] | 216 | exceptions and the later is a list of autotest/autoserv internal exceptions. Of |
| 217 | course, if you really need to, you can extend the exception definitions on |
Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 218 | client/common\_lib/error.py. |
mbligh | 900b6c1 | 2008-01-14 16:56:47 +0000 | [diff] [blame] | 219 | |
| 220 | |
Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 221 | ## Submitting patches |
mbligh | 71d338a | 2007-10-08 05:05:50 +0000 | [diff] [blame] | 222 | |
Christopher Wiley | 2f011d9 | 2016-02-04 09:35:32 -0800 | [diff] [blame] | 223 | Submit changes through the Chrome OS gerrit instance. This process is |
| 224 | documented on |
| 225 | [chromium.org](http://dev.chromium.org/developers/contributing-code). |