| 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 | Leave TWO blank lines between functions - this is Python, there are no clear |
| Christopher Wiley | 2f011d9 | 2016-02-04 09:35:32 -0800 | [diff] [blame^] | 45 | function end markers, and we need help. Note that this intentionally |
| 46 | contradicts PEP-8. |
| mbligh | 71d338a | 2007-10-08 05:05:50 +0000 | [diff] [blame] | 47 | |
| 48 | |
| Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 49 | ## Variable names and UpPeR cAsE |
| Christopher Wiley | 2f011d9 | 2016-02-04 09:35:32 -0800 | [diff] [blame^] | 50 | * Use descriptive variable names where possible |
| 51 | * Use `variable_names_like_this` |
| 52 | * Use `method_and_function_names_like_this` |
| 53 | * Use `UpperCamelCase` for class names |
| mbligh | 71d338a | 2007-10-08 05:05:50 +0000 | [diff] [blame] | 54 | |
| mbligh | d876f45 | 2008-12-03 15:09:17 +0000 | [diff] [blame] | 55 | |
| Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 56 | ## Importing modules |
| mbligh | 7654c82 | 2008-04-04 15:12:48 +0000 | [diff] [blame] | 57 | |
| 58 | The order of imports should be as follows: |
| 59 | |
| Christopher Wiley | 2f011d9 | 2016-02-04 09:35:32 -0800 | [diff] [blame^] | 60 | * Standard python modules |
| 61 | * Non-standard python modules |
| 62 | * Autotest modules |
| mbligh | 7654c82 | 2008-04-04 15:12:48 +0000 | [diff] [blame] | 63 | |
| 64 | Within one of these three sections, all module imports using the from |
| 65 | keyword should appear after regular imports. |
| Christopher Wiley | d7445ef | 2014-12-05 13:26:05 -0800 | [diff] [blame] | 66 | Each module should be imported on its own line. |
| Christopher Wiley | 2f011d9 | 2016-02-04 09:35:32 -0800 | [diff] [blame^] | 67 | Do not use Wildcard imports (`from x import *`) if possible. |
| 68 | |
| 69 | Import modules, not classes. For example: |
| Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 70 | |
| 71 | ``` |
| mbligh | 7654c82 | 2008-04-04 15:12:48 +0000 | [diff] [blame] | 72 | from common_lib import error |
| Christopher Wiley | 2f011d9 | 2016-02-04 09:35:32 -0800 | [diff] [blame^] | 73 | |
| 74 | def foo(): |
| 75 | raise error.AutoservError(...) |
| Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 76 | ``` |
| 77 | |
| 78 | and not: |
| 79 | |
| 80 | ``` |
| mbligh | 7654c82 | 2008-04-04 15:12:48 +0000 | [diff] [blame] | 81 | from common_lib.error import AutoservError |
| Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 82 | ``` |
| mbligh | 7654c82 | 2008-04-04 15:12:48 +0000 | [diff] [blame] | 83 | |
| 84 | For example: |
| Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 85 | |
| 86 | ``` |
| Christopher Wiley | d7445ef | 2014-12-05 13:26:05 -0800 | [diff] [blame] | 87 | import os |
| 88 | import pickle |
| 89 | import random |
| 90 | import re |
| 91 | import select |
| 92 | import shutil |
| 93 | import signal |
| 94 | import subprocess |
| 95 | |
| mbligh | 8bcd23a | 2009-02-03 19:14:06 +0000 | [diff] [blame] | 96 | import common # Magic autotest_lib module and sys.path setup code. |
| 97 | import MySQLdb # After common so that we check our site-packages first. |
| Christopher Wiley | d7445ef | 2014-12-05 13:26:05 -0800 | [diff] [blame] | 98 | |
| mbligh | 7654c82 | 2008-04-04 15:12:48 +0000 | [diff] [blame] | 99 | from common_lib import error |
| Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 100 | ``` |
| mbligh | 7654c82 | 2008-04-04 15:12:48 +0000 | [diff] [blame] | 101 | |
| Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 102 | ## Testing None |
| mbligh | d876f45 | 2008-12-03 15:09:17 +0000 | [diff] [blame] | 103 | |
| Christopher Wiley | 2f011d9 | 2016-02-04 09:35:32 -0800 | [diff] [blame^] | 104 | 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] | 105 | 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^] | 106 | method does the wrong thing. |
| mbligh | d876f45 | 2008-12-03 15:09:17 +0000 | [diff] [blame] | 107 | |
| mbligh | 71d338a | 2007-10-08 05:05:50 +0000 | [diff] [blame] | 108 | |
| Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 109 | ## Comments |
| mbligh | 71d338a | 2007-10-08 05:05:50 +0000 | [diff] [blame] | 110 | |
| 111 | Generally, you want your comments to tell WHAT your code does, not HOW. |
| 112 | We can figure out how from the code itself (or if not, your code needs fixing). |
| 113 | |
| 114 | Try to describle the intent of a function and what it does in a triple-quoted |
| 115 | (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] | 116 | places, though undoubtedly we're not perfect. A high level overview is |
| mbligh | 71d338a | 2007-10-08 05:05:50 +0000 | [diff] [blame] | 117 | incredibly helpful in understanding code. |
| 118 | |
| 119 | |
| Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 120 | ## Hardcoded String Formatting |
| Simran Basi | c7330bd | 2013-05-31 09:23:50 -0700 | [diff] [blame] | 121 | |
| 122 | Strings should use only single quotes for hardcoded strings in the code. Double |
| 123 | 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^] | 124 | Multiline strings should not use '\' but wrap the string using parentheseses. |
| Simran Basi | c7330bd | 2013-05-31 09:23:50 -0700 | [diff] [blame] | 125 | |
| 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 | REALLY_LONG_STRING = ('This is supposed to be a really long string that is ' |
| 128 | 'over 80 characters and does not use a slash to ' |
| 129 | 'continue.') |
| Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 130 | ``` |
| Simran Basi | c7330bd | 2013-05-31 09:23:50 -0700 | [diff] [blame] | 131 | |
| Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 132 | ## Docstrings |
| mbligh | 5cad50f | 2009-06-08 16:50:51 +0000 | [diff] [blame] | 133 | |
| 134 | Docstrings are important to keep our code self documenting. While it's not |
| 135 | necessary to overdo documentation, we ask you to be sensible and document any |
| 136 | nontrivial function. When creating docstrings, please add a newline at the |
| showard | 25f056a | 2009-11-23 20:22:50 +0000 | [diff] [blame] | 137 | beginning of your triple quoted string and another newline at the end of it. If |
| 138 | the docstring has multiple lines, please include a short summary line followed |
| 139 | by a blank line before continuing with the rest of the description. Please |
| 140 | capitalize and punctuate accordingly the sentences. If the description has |
| 141 | multiple lines, put two levels of indentation before proceeding with text. An |
| 142 | example docstring: |
| mbligh | 5cad50f | 2009-06-08 16:50:51 +0000 | [diff] [blame] | 143 | |
| Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 144 | ``` |
| mbligh | 5cad50f | 2009-06-08 16:50:51 +0000 | [diff] [blame] | 145 | def foo(param1, param2): |
| mbligh | 5220736 | 2009-09-03 20:54:29 +0000 | [diff] [blame] | 146 | """ |
| showard | 25f056a | 2009-11-23 20:22:50 +0000 | [diff] [blame] | 147 | Summary line. |
| 148 | |
| mbligh | 5220736 | 2009-09-03 20:54:29 +0000 | [diff] [blame] | 149 | Long description of method foo. |
| mbligh | 5cad50f | 2009-06-08 16:50:51 +0000 | [diff] [blame] | 150 | |
| mbligh | 5220736 | 2009-09-03 20:54:29 +0000 | [diff] [blame] | 151 | @param param1: A thing called param1 that is used for a bunch of stuff |
| 152 | that has methods bar() and baz() which raise SpamError if |
| 153 | something goes awry. |
| Simran Basi | c7330bd | 2013-05-31 09:23:50 -0700 | [diff] [blame] | 154 | |
| 155 | @returns a list of integers describing changes in a source tree |
| 156 | |
| 157 | @raises exception that could be raised if a certain condition occurs. |
| 158 | |
| mbligh | 5220736 | 2009-09-03 20:54:29 +0000 | [diff] [blame] | 159 | """ |
| Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 160 | ``` |
| mbligh | 5cad50f | 2009-06-08 16:50:51 +0000 | [diff] [blame] | 161 | |
| 162 | The tags that you can put inside your docstring are tags recognized by systems |
| 163 | like doxygen. Not all places need all tags defined, so choose them wisely while |
| Simran Basi | c7330bd | 2013-05-31 09:23:50 -0700 | [diff] [blame] | 164 | writing code. Generally (if applicable) always list parameters, return value |
| 165 | (if there is one), and exceptions that can be raised to each docstring. |
| mbligh | 5cad50f | 2009-06-08 16:50:51 +0000 | [diff] [blame] | 166 | |
| Christopher Wiley | 2f011d9 | 2016-02-04 09:35:32 -0800 | [diff] [blame^] | 167 | | Tag | Description | |
| 168 | |----------|------------------------------------------------------------------------------------------| |
| 169 | | @author | Code author | |
| 170 | | @param | Parameter description | |
| 171 | | @raise | If the function can throw an exception, this tag documents the possible exception types. | |
| 172 | | @raises | Same as @raise. | |
| 173 | | @return | Return value description | |
| 174 | | @returns | Same as @return | |
| 175 | | @see | Reference to other parts of the codebase. | |
| 176 | | @warning | Call attention to potential problems with the code | |
| 177 | | @var | Documentation for a variable or enum value (either global or as a member of a class) | |
| 178 | | @version | Version string | |
| mbligh | 5cad50f | 2009-06-08 16:50:51 +0000 | [diff] [blame] | 179 | |
| mbligh | 3bdba92 | 2010-05-03 18:02:54 +0000 | [diff] [blame] | 180 | When in doubt refer to: http://doxygen.nl/commands.html |
| 181 | |
| Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 182 | ## Simple code |
| mbligh | 71d338a | 2007-10-08 05:05:50 +0000 | [diff] [blame] | 183 | |
| 184 | Keep it simple; this is not the right place to show how smart you are. We |
| 185 | have plenty of system failures to deal with without having to spend ages |
| 186 | figuring out your code, thanks ;-) Readbility, readability, readability. |
| Christopher Wiley | 2f011d9 | 2016-02-04 09:35:32 -0800 | [diff] [blame^] | 187 | Strongly prefer readability and simplicity over compactness. |
| mbligh | 71d338a | 2007-10-08 05:05:50 +0000 | [diff] [blame] | 188 | |
| 189 | "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] | 190 | 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] | 191 | smart enough to debug it." Brian Kernighan |
| 192 | |
| 193 | |
| Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 194 | ## Function length |
| mbligh | 71d338a | 2007-10-08 05:05:50 +0000 | [diff] [blame] | 195 | |
| 196 | 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^] | 197 | you are amazingly clever, and can cope with it, the rest of us are busy and |
| 198 | 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] | 199 | |
| 200 | Functions should be short and sweet, and do just one thing. They should |
| 201 | fit on one or two screenfuls of text (the ISO/ANSI screen size is 80x24, |
| 202 | as we all know), and do one thing and do that well. |
| 203 | |
| 204 | |
| Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 205 | ## Exceptions |
| mbligh | 900b6c1 | 2008-01-14 16:56:47 +0000 | [diff] [blame] | 206 | |
| 207 | When raising exceptions, the preferred syntax for it is: |
| 208 | |
| Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 209 | ``` |
| mbligh | 900b6c1 | 2008-01-14 16:56:47 +0000 | [diff] [blame] | 210 | raise FooException('Exception Message') |
| Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 211 | ``` |
| mbligh | 900b6c1 | 2008-01-14 16:56:47 +0000 | [diff] [blame] | 212 | |
| 213 | Please don't raise string exceptions, as they're deprecated and will be removed |
| 214 | from future versions of python. If you're in doubt about what type of exception |
| 215 | 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] | 216 | 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] | 217 | exceptions and the later is a list of autotest/autoserv internal exceptions. Of |
| 218 | 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] | 219 | client/common\_lib/error.py. |
| mbligh | 900b6c1 | 2008-01-14 16:56:47 +0000 | [diff] [blame] | 220 | |
| 221 | |
| Christopher Wiley | fe2ea87 | 2016-02-02 12:12:18 -0800 | [diff] [blame] | 222 | ## Submitting patches |
| mbligh | 71d338a | 2007-10-08 05:05:50 +0000 | [diff] [blame] | 223 | |
| Christopher Wiley | 2f011d9 | 2016-02-04 09:35:32 -0800 | [diff] [blame^] | 224 | Submit changes through the Chrome OS gerrit instance. This process is |
| 225 | documented on |
| 226 | [chromium.org](http://dev.chromium.org/developers/contributing-code). |