docs/test_code_guidelines.rst

   1 CSIT Test Code Guidelines
   2 ^^^^^^^^^^^^^^^^^^^^^^^^^
   3
   4 The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
   5 "SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED",
   6 "MAY", and "OPTIONAL" in this document are to be interpreted as
   7 described in `BCP 14 <https://tools.ietf.org/html/bcp14>`_
   8 `[RFC2119] <https://tools.ietf.org/html/rfc2119>`_
   9 `[RFC8174] <https://tools.ietf.org/html/rfc8174>`_
  10 when, and only when, they appear in all capitals, as shown here.
  11
  12 This document SHALL describe guidelines for writing reliable, maintainable,
  13 reusable and readable code for CSIT.
  14
  15 TODO: Decide whether to use "you SHALL", "contributors SHALL",
  16 or "code SHALL be"; convert other forms to the chosen one.
  17
  18 RobotFramework test case files and resource files
  19 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  20
  21 + General
  22
  23   + Contributors SHOULD look at requirements.txt in root CSIT directory
  24     for the currently used Robot Framework version.
  25     Contributors SHOULD read `Robot Framework User Guide
  26     <http://robotframework.org/robotframework/latest/RobotFrameworkUserGuide.html>`_
  27     for more details.
  28
  29   + RobotFramework test case files and resource files
  30     SHALL use special extension .robot
  31
  32   + Pipe and space separated file format (without trailing pipe
  33     and without pipe aligning) SHALL be used.
  34     Tabs are invisible characters, which are error prone.
  35     4-spaces separation is prone to accidental double space
  36     acting as a separator.
  37
  38   + Files SHALL be encoded in UTF-8 (the default Robot source file encoding).
  39     Usage of non-ASCII characters SHOULD be avoided if possible.
  40     It is RECOMMENDED to `escape
  41     <http://robotframework.org/robotframework/latest/RobotFrameworkUserGuide.html#escaping>`_
  42     non-ASCII characters.
  43
  44   + Line length SHALL be limited to 80 characters.
  45
  46   + There SHALL be licence text (FIXME: add link) present
  47     at the beginning of each file.
  48
  49   + Copy-pasting of the code NOT RECOMMENDED practice, any code that could be
  50     re-used SHOULD be put into a library (Robot resource, Python library, ...).
  51
  52 + Test cases
  53
  54   + It is RECOMMENDED to use data-driven test case definitions
  55     anytime suite contains test cases similar in structure.
  56     Typically, a suite SHOULD define a Template keyword, and test cases
  57     SHOULD only specify tags and argument values::
  58
  59         *** Settings ***
  60         | Test Template | Local Template
  61         ...
  62
  63         *** Test Cases ***
  64         | tc01-64B-1c-eth-l2patch-mrr
  65         | | [Tags] | 64B | 1C
  66         | | framesize=${64} | phy_cores=${1}
  67
  68   + Test case templates (or testcases) SHALL be written in Behavior-driven style
  69     i.e. in readable English, so that even non-technical project stakeholders
  70     can understand it::
  71
  72         *** Keywords ***
  73         | Local Template
  74         | | [Documentation]
  75         | | ... | [Cfg] DUT runs L2 patch config with ${phy_cores} phy
  76         | | ... | core(s).
  77         | | ... | [Ver] Measure MaxReceivedRate for ${framesize}B frames\
  78         | | ... | using single trial throughput test.
  79         | | ...
  80         | | ... | *Arguments:*
  81         | | ... | - framesize - Framesize in Bytes in integer\
  82         | | ... |   or string (IMIX_v4_1). Type: integer, string
  83         | | ... | - phy_cores - Number of physical cores. Type: integer
  84         | | ... | - rxq - Number of RX queues, default value: ${None}.\
  85         | | ... |   Type: integer
  86         | | ...
  87         | | [Arguments] | ${framesize} | ${phy_cores} | ${rxq}=${None}
  88         | | ...
  89         | | Given Add worker threads and rxqueues to all DUTs
  90         | | ... | ${phy_cores} | ${rxq}
  91         | | And Add PCI devices to all DUTs
  92         | | ${max_rate} | ${jumbo} = | Run Keyword
  93         | | ... | Get Max Rate And Jumbo And Handle Multi Seg
  94         | | ... | ${s_24.5G} | ${framesize} | pps_limit=${s_18.75Mpps}
  95         | | And Apply startup configuration on all VPP DUTs
  96         | | When Initialize L2 patch
  97         | | Then Traffic should pass with maximum rate
  98         | | ... | ${max_rate}pps | ${framesize} | ${traffic_profile}
  99
 100   + Every suite and test case template (or testcase)
 101     SHALL contain short documentation.
 102     Generated CSIT web pages display the documentation.
 103     For an example generated page, see:
 104     https://docs.fd.io/csit/rls1807/doc/tests.vpp.perf.tcp.html
 105
 106   + You SHOULD NOT use hard-coded constants.
 107     It is RECOMMENDED to use the variable table
 108     (\*\*\*Variables\*\*\*) to define test case specific values.
 109     You SHALL use the assignment sign = after the variable name
 110     to make assigning variables slightly more explicit::
 111
 112         *** Variables ***
 113         | ${traffic_profile}= | trex-sl-2n-ethip4-ip4src254
 114
 115   + Common test case specific settings of the test environment SHALL be done
 116     in Test Setup keyword defined in the Setting table.
 117
 118     + Run Keywords construction is RECOMMENDED if it is more readable
 119       than a keyword.
 120
 121     + Separate keyword is RECOMMENDED if the construction is less readable.
 122
 123   + Post-test cleaning and processing actions SHALL be done in Test Teardown
 124     part of the Setting table (e.g. download statistics from VPP nodes).
 125     This part is executed even if the test case has failed. On the other hand
 126     it is possible to disable the tear-down from command line, thus leaving
 127     the system in “broken” state for investigation.
 128
 129   + Every testcase SHALL be correctly tagged. List of defined tags is in
 130     csit/docs/tag_documentation.rst (FIXME: rst-ize the link) file.
 131
 132     + Whenever possible, common tags SHALL be set using Force Tags
 133       in Settings table.
 134
 135   + User high-level keywords specific for the particular test suite
 136     SHOULD be implemented in the Keywords table of suitable Robot resource file
 137     to enable readability and code-reuse.
 138
 139     + Such keywords MAY be implemented in Keywords table of the suite instead,
 140       if the contributor believes no other test will use such keywords.
 141       But this is NOT RECOMMENDED in general, as keywords in Resources
 142       are easier to maintain.
 143
 144   + All test case names (and suite names) SHALL conform
 145     to current naming convention.
 146     https://wiki.fd.io/view/CSIT/csit-test-naming
 147     TODO: Migrate the convention document to .rst and re-link.
 148
 149   + Frequently, different suites use the same test case layout.
 150     It is RECOMMENDED to use autogeneration scripts available,
 151     possibly extending them if their current functionality is not sufficient.
 152
 153 + Resource files
 154
 155   + SHALL be used to implement higher-level keywords that are used in test cases
 156     or other higher-level (or medium-level) keywords.
 157
 158   + Every keyword SHALL contain Documentation where the purpose and arguments
 159     of the keyword are described. Also document types, return values,
 160     and any specific assumptions the particular keyword relies on.
 161
 162   + A keyword usage example SHALL be the part of the Documentation.
 163     The example SHALL use pipe and space separated format
 164     (with escaped pipes and) with a trailing pipe.
 165
 166     + The reason was possbile usage of Robot's libdoc tool
 167       to generate tests and resources documentation. In that case
 168       example keyword usage would be rendered in table.
 169
 170     + TODO: We should adapt it for current tool
 171       used to generate the documentation.
 172
 173   + Keyword name SHALL describe what the keyword does,
 174     specifically and in a reasonable length (“short sentence”).
 175
 176     + Keyword names SHALL be short enough for call sites
 177       to fit within line length limit.
 178
 179   + If a keyword argument has a most commonly used value, it is RECOMMENDED
 180     to set it as default. This makes keyword code longer,
 181     but suite code shorter, and readability (and maintainability)
 182     of suites SHALL always more important.
 183
 184   + If there is intermediate data (created by one keyword, to be used
 185     by another keyword) of singleton semantics (it is clear that the test case
 186     can have at most one instance of such data, even if the instance
 187     is complex, for example ${nodes}), it is RECOMMENDED to store it
 188     in test variables. You SHALL document test variables read or written
 189     by a keyword. This makes the test template code less verbose.
 190     As soon as the data instance is not unique, you SHALL pass it around
 191     via arguments and return values explicitly (this makes lower level keywords
 192     more reusable and less bug prone).
 193
 194   + It is RECOMMENDED to pass arguments explicitly via [Arguments] line.
 195     Setting test variables takes more space and is less explicit.
 196     Using arguments embedded in keyword name makes them less visible,
 197     and it makes it harder for the line containing the resulting long name
 198     to fit into the maximum character limit, so you SHOULD NOT use them.
 199
 200 Python library files
 201 ~~~~~~~~~~~~~~~~~~~~
 202
 203 TODO: Add guidelines for Python scripts (both utilities called by test on nodes
 204 and unrelated ones such as PAL) if there are any (in addition to library ones).
 205
 206 + General
 207
 208   + SHALL be used to implement low-level keywords that are called from
 209     resource files (of higher-level keywords) or from test cases.
 210
 211   + TODO: Discuss debugability, speed, logging, complexity of logic.
 212
 213   + Higher-level keywords MAY be implemented in python library file too.
 214     it is RECOMMENDED especially in the case that their implementation
 215     in resource file would be too difficult or impossible,
 216     e.g. complex data structures or functional programming.
 217
 218   + Every keyword, Python module, class, method, enum SHALL contain
 219     docstring with the short description and used input parameters
 220     and possible return value(s) or raised exceptions.
 221
 222     + The docstrings SHOULD conform to
 223       `PEP 257 <https://www.python.org/dev/peps/pep-0257/>`_
 224       and other quality standards.
 225
 226     + CSIT contributions SHALL use a specific formatting for documenting
 227       arguments, return values and similar.
 228
 229       + FIXME: Find a link which documents sthis style.
 230         it is based on Sphinx, but very different from
 231         `Napoleon style
 232         <https://sphinxcontrib-napoleon.readthedocs.io/en/latest/example_numpy.html>`_.
 233
 234   + Keyword usage examples MAY be grouped and used
 235     in the class/module documentation string, to provide better overview
 236     of the usage and relationships between keywords.
 237
 238   + Keyword name SHALL describe what the keyword does,
 239     specifically and in a reasonable length (“short sentence”).
 240     See https://wiki.fd.io/view/CSIT/csit-test-naming
 241
 242   + Python implementation of a keyword is a function,
 243     so its name in the python library should be lowercase_with_underscores.
 244     Robot call sites should usename with first letter capitalized, and spaces.
 245
 246     + FIXME: create Robot keyword naming item in proper place.
 247
 248 + Coding
 249
 250   + It is RECOMMENDED to use some standard development tool
 251     (e.g. PyCharm Community Edition) and follow
 252     `PEP-8 <https://www.python.org/dev/peps/pep-0008/>`_ recommendations.
 253
 254   + All python code (not only Robot libraries) SHALL adhere to PEP-8 standard.
 255     This is reported by CSIT Jenkins verify job.
 256
 257   + Indentation: You SHALL NOT use tab for indents!
 258     Indent is defined as four spaces.
 259
 260   + Line length: SHALL be limited to 80 characters.
 261
 262   + CSIT Python code assumes PYTHONPATH is set
 263     to the root of cloned CSIT git repository, creating a tree of sub-packages.
 264     You SHALL use that tree for importing, for example::
 265
 266        from resources.libraries.python.ssh import exec_cmd_no_error
 267
 268   + Imports SHALL be grouped in the following order:
 269
 270       #. standard library imports,
 271       #. related third party imports,
 272       #. local application/library specific imports.
 273
 274     You SHALL put a blank line between each group of imports.
 275
 276   + You SHALL use two blank lines between top-level definitions,
 277     one blank line between method definitions.
 278
 279   + You SHALL NOT execute any active code on library import.
 280
 281   + You SHALL NOT use global variables inside library files.
 282
 283     + You MAY define constants inside library files.
 284
 285   + It is NOT RECOMMENDED to use hard-coded constants (e.g. numbers,
 286     paths without any description). It is RECOMMENDED to use
 287     configuration file(s), like /csit/resources/libraries/python/constants.py,
 288     with appropriate comments.
 289
 290   + The code SHALL log at the lowest possible level of implementation,
 291     for debugging purposes. You SHALL use same style for similar events.
 292     You SHALL keep logging as verbose as necessary.
 293
 294   + You SHALL use the most appropriate exception not general one (Exception)
 295     if possible. You SHOULD create your own exception
 296     if necessary and implement there logging, level debug.
 297
 298     + You MAY use RuntimeException for generally unexpected failures.
 299
 300     + It is RECOMMENDED to use RuntimeError also for
 301       infrastructure failures, e.g. losing SSH connection to SUT.
 302
 303       + You MAY use EnvironmentError and its cublasses instead,
 304         if the distinction is informative for callers.
 305
 306     + It is RECOMMENDED to use AssertionError when SUT is at fault.
 307
 308   + For each class (e.g. exception) it is RECOMMENDED to implement __repr__()
 309     which SHALL return a string usable as a constructor call
 310     (including repr()ed arguments).
 311     When logging, you SHOULD log the repr form, unless the internal structure
 312     of the object in question would likely result in too long output.
 313     This is helpful for debugging.
 314
 315   + For composing and formatting strings, you SHOULD use .format()
 316     with named arguments.
 317     Example: "repr() of name: {name!r}".format(name=name)
 318
 319 Bash scripts and libraries
 320 ~~~~~~~~~~~~~~~~~~~~~~~~~~
 321
 322 TODO: Link here when document for this is ready.