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 core(s).
  76         | | ... | [Ver] Measure NDR and PDR values using MLRsearch algorithm.\
  77         | | ...
  78         | | ... | *Arguments:*
  79         | | ... | - frame_size - Framesize in Bytes in integer
  80         | | ... | or string (IMIX_v4_1). Type: integer, string
  81         | | ... | - phy_cores - Number of physical cores. Type: integer
  82         | | ... | - rxq - Number of RX queues, default value: ${None}.
  83         | | ... | Type: integer
  84         | | ...
  85         | | [Arguments] | ${frame_size} | ${phy_cores} | ${rxq}=${None}
  86         | | ...
  87         | | Set Test Variable | \${frame_size}
  88         | | ...
  89         | | Given Add worker threads and rxqueues to all DUTs
  90         | | ... | ${phy_cores} | ${rxq}
  91         | | And Add PCI devices to all DUTs
  92         | | Set Max Rate And Jumbo And Handle Multi Seg
  93         | | And Apply startup configuration on all VPP DUTs
  94         | | When Initialize L2 patch
  95         | | Then Find NDR and PDR intervals using optimized search
  96
  97   + Every suite and test case template (or testcase)
  98     SHALL contain short documentation.
  99     Generated CSIT web pages display the documentation.
 100     For an example generated page, see:
 101     https://docs.fd.io/csit/rls1807/doc/tests.vpp.perf.tcp.html
 102
 103   + You SHOULD NOT use hard-coded constants.
 104     It is RECOMMENDED to use the variable table
 105     (\*\*\*Variables\*\*\*) to define test case specific values.
 106     You SHALL use the assignment sign = after the variable name
 107     to make assigning variables slightly more explicit::
 108
 109         *** Variables ***
 110         | ${traffic_profile}= | trex-stl-2n-ethip4-ip4src254
 111
 112   + Common test case specific settings of the test environment SHALL be done
 113     in Test Setup keyword defined in the Setting table.
 114
 115     + Run Keywords construction is RECOMMENDED if it is more readable
 116       than a keyword.
 117
 118     + Separate keyword is RECOMMENDED if the construction is less readable.
 119
 120   + Post-test cleaning and processing actions SHALL be done in Test Teardown
 121     part of the Setting table (e.g. download statistics from VPP nodes).
 122     This part is executed even if the test case has failed. On the other hand
 123     it is possible to disable the tear-down from command line, thus leaving
 124     the system in “broken” state for investigation.
 125
 126   + Every testcase SHALL be correctly tagged. List of defined tags is in
 127     csit/docs/tag_documentation.rst (FIXME: rst-ize the link) file.
 128
 129     + Whenever possible, common tags SHALL be set using Force Tags
 130       in Settings table.
 131
 132   + User high-level keywords specific for the particular test suite
 133     SHOULD be implemented in the Keywords table of suitable Robot resource file
 134     to enable readability and code-reuse.
 135
 136     + Such keywords MAY be implemented in Keywords table of the suite instead,
 137       if the contributor believes no other test will use such keywords.
 138       But this is NOT RECOMMENDED in general, as keywords in Resources
 139       are easier to maintain.
 140
 141   + All test case names (and suite names) SHALL conform
 142     to current naming convention.
 143     https://wiki.fd.io/view/CSIT/csit-test-naming
 144     TODO: Migrate the convention document to .rst and re-link.
 145
 146   + Frequently, different suites use the same test case layout.
 147     It is RECOMMENDED to use autogeneration scripts available,
 148     possibly extending them if their current functionality is not sufficient.
 149
 150 + Resource files
 151
 152   + SHALL be used to implement higher-level keywords that are used in test cases
 153     or other higher-level (or medium-level) keywords.
 154
 155   + Every keyword SHALL contain Documentation where the purpose and arguments
 156     of the keyword are described. Also document types, return values,
 157     and any specific assumptions the particular keyword relies on.
 158
 159   + A keyword usage example SHALL be the part of the Documentation.
 160     The example SHALL use pipe and space separated format
 161     (with escaped pipes and) with a trailing pipe.
 162
 163     + The reason was possbile usage of Robot's libdoc tool
 164       to generate tests and resources documentation. In that case
 165       example keyword usage would be rendered in table.
 166
 167     + TODO: We should adapt it for current tool
 168       used to generate the documentation.
 169
 170   + Keyword name SHALL describe what the keyword does,
 171     specifically and in a reasonable length (“short sentence”).
 172
 173     + Keyword names SHALL be short enough for call sites
 174       to fit within line length limit.
 175
 176   + If a keyword argument has a most commonly used value, it is RECOMMENDED
 177     to set it as default. This makes keyword code longer,
 178     but suite code shorter, and readability (and maintainability)
 179     of suites SHALL always more important.
 180
 181   + If there is intermediate data (created by one keyword, to be used
 182     by another keyword) of singleton semantics (it is clear that the test case
 183     can have at most one instance of such data, even if the instance
 184     is complex, for example ${nodes}), it is RECOMMENDED to store it
 185     in test variables. You SHALL document test variables read or written
 186     by a keyword. This makes the test template code less verbose.
 187     As soon as the data instance is not unique, you SHALL pass it around
 188     via arguments and return values explicitly (this makes lower level keywords
 189     more reusable and less bug prone).
 190
 191   + It is RECOMMENDED to pass arguments explicitly via [Arguments] line.
 192     Setting test variables takes more space and is less explicit.
 193     Using arguments embedded in keyword name makes them less visible,
 194     and it makes it harder for the line containing the resulting long name
 195     to fit into the maximum character limit, so you SHOULD NOT use them.
 196
 197 Python library files
 198 ~~~~~~~~~~~~~~~~~~~~
 199
 200 TODO: Add guidelines for Python scripts (both utilities called by test on nodes
 201 and unrelated ones such as PAL) if there are any (in addition to library ones).
 202
 203 + General
 204
 205   + SHALL be used to implement low-level keywords that are called from
 206     resource files (of higher-level keywords) or from test cases.
 207
 208   + TODO: Discuss debugability, speed, logging, complexity of logic.
 209
 210   + Higher-level keywords MAY be implemented in python library file too.
 211     it is RECOMMENDED especially in the case that their implementation
 212     in resource file would be too difficult or impossible,
 213     e.g. complex data structures or functional programming.
 214
 215   + Every keyword, Python module, class, method, enum SHALL contain
 216     docstring with the short description and used input parameters
 217     and possible return value(s) or raised exceptions.
 218
 219     + The docstrings SHOULD conform to
 220       `PEP 257 <https://www.python.org/dev/peps/pep-0257/>`_
 221       and other quality standards.
 222
 223     + CSIT contributions SHALL use a specific formatting for documenting
 224       arguments, return values and similar.
 225
 226       + FIXME: Find a link which documents sthis style.
 227         it is based on Sphinx, but very different from
 228         `Napoleon style
 229         <https://sphinxcontrib-napoleon.readthedocs.io/en/latest/example_numpy.html>`_.
 230
 231   + Keyword usage examples MAY be grouped and used
 232     in the class/module documentation string, to provide better overview
 233     of the usage and relationships between keywords.
 234
 235   + Keyword name SHALL describe what the keyword does,
 236     specifically and in a reasonable length (“short sentence”).
 237     See https://wiki.fd.io/view/CSIT/csit-test-naming
 238
 239   + Python implementation of a keyword is a function,
 240     so its name in the python library should be lowercase_with_underscores.
 241     Robot call sites should usename with first letter capitalized, and spaces.
 242
 243     + FIXME: create Robot keyword naming item in proper place.
 244
 245 + Coding
 246
 247   + It is RECOMMENDED to use some standard development tool
 248     (e.g. PyCharm Community Edition) and follow
 249     `PEP-8 <https://www.python.org/dev/peps/pep-0008/>`_ recommendations.
 250
 251   + All python code (not only Robot libraries) SHALL adhere to PEP-8 standard.
 252     This is reported by CSIT Jenkins verify job.
 253
 254   + Indentation: You SHALL NOT use tab for indents!
 255     Indent is defined as four spaces.
 256
 257   + Line length: SHALL be limited to 80 characters.
 258
 259   + CSIT Python code assumes PYTHONPATH is set
 260     to the root of cloned CSIT git repository, creating a tree of sub-packages.
 261     You SHALL use that tree for importing, for example::
 262
 263        from resources.libraries.python.ssh import exec_cmd_no_error
 264
 265   + Imports SHALL be grouped in the following order:
 266
 267       #. standard library imports,
 268       #. related third party imports,
 269       #. local application/library specific imports.
 270
 271     You SHALL put a blank line between each group of imports.
 272
 273   + You SHALL use two blank lines between top-level definitions,
 274     one blank line between method definitions.
 275
 276   + You SHALL NOT execute any active code on library import.
 277
 278   + You SHALL NOT use global variables inside library files.
 279
 280     + You MAY define constants inside library files.
 281
 282   + It is NOT RECOMMENDED to use hard-coded constants (e.g. numbers,
 283     paths without any description). It is RECOMMENDED to use
 284     configuration file(s), like /csit/resources/libraries/python/Constants.py,
 285     with appropriate comments.
 286
 287   + The code SHALL log at the lowest possible level of implementation,
 288     for debugging purposes. You SHALL use same style for similar events.
 289     You SHALL keep logging as verbose as necessary.
 290
 291   + You SHALL use the most appropriate exception not general one (Exception)
 292     if possible. You SHOULD create your own exception
 293     if necessary and implement there logging, level debug.
 294
 295     + You MAY use RuntimeException for generally unexpected failures.
 296
 297     + It is RECOMMENDED to use RuntimeError also for
 298       infrastructure failures, e.g. losing SSH connection to SUT.
 299
 300       + You MAY use EnvironmentError and its cublasses instead,
 301         if the distinction is informative for callers.
 302
 303     + It is RECOMMENDED to use AssertionError when SUT is at fault.
 304
 305   + For each class (e.g. exception) it is RECOMMENDED to implement __repr__()
 306     which SHALL return a string usable as a constructor call
 307     (including repr()ed arguments).
 308     When logging, you SHOULD log the repr form, unless the internal structure
 309     of the object in question would likely result in too long output.
 310     This is helpful for debugging.
 311
 312   + For composing and formatting strings, you SHOULD use .format()
 313     with named arguments.
 314     Example: "repr() of name: {name!r}".format(name=name)
 315
 316 Bash scripts and libraries
 317 ~~~~~~~~~~~~~~~~~~~~~~~~~~
 318
 319 TODO: Link or copy the bash_code_style.rst document here.