| Attila Fazekas | 23fdf1d | 2013-06-09 16:35:23 +0200 | [diff] [blame] | 1 | Tempest Coding Guide | 
|  | 2 | ==================== | 
|  | 3 |  | 
| Joe Gordon | 1374f88 | 2013-07-12 17:00:34 +0100 | [diff] [blame] | 4 | - Step 1: Read the OpenStack Style Commandments | 
| chenxing | e98720a | 2017-07-19 03:42:23 +0000 | [diff] [blame] | 5 | https://docs.openstack.org/hacking/latest/ | 
| Joe Gordon | 1374f88 | 2013-07-12 17:00:34 +0100 | [diff] [blame] | 6 | - Step 2: Read on | 
|  | 7 |  | 
|  | 8 | Tempest Specific Commandments | 
| Sergey Vilgelm | eac094a | 2018-11-21 18:27:51 -0600 | [diff] [blame] | 9 | ----------------------------- | 
| Joe Gordon | 1374f88 | 2013-07-12 17:00:34 +0100 | [diff] [blame] | 10 |  | 
| ghanshyam | 50f1947 | 2014-11-26 17:04:37 +0900 | [diff] [blame] | 11 | - [T102] Cannot import OpenStack python clients in tempest/api & | 
| Masayuki Igawa | b78b923 | 2017-11-17 16:12:37 +0900 | [diff] [blame] | 12 | tempest/scenario tests | 
| Matthew Treinish | 5e4c0f2 | 2013-09-10 18:38:28 +0000 | [diff] [blame] | 13 | - [T104] Scenario tests require a services decorator | 
| Andrea Frittoli | a5ddd55 | 2014-08-19 18:30:00 +0100 | [diff] [blame] | 14 | - [T105] Tests cannot use setUpClass/tearDownClass | 
| Ken'ichi Ohmichi | 7581bcd | 2015-02-16 04:09:58 +0000 | [diff] [blame] | 15 | - [T107] Check that a service tag isn't in the module path | 
| Ken'ichi Ohmichi | 80369a9 | 2015-04-06 23:41:14 +0000 | [diff] [blame] | 16 | - [T108] Check no hyphen at the end of rand_name() argument | 
| John Warren | 3059a09 | 2015-08-31 15:34:49 -0400 | [diff] [blame] | 17 | - [T109] Cannot use testtools.skip decorator; instead use | 
| Masayuki Igawa | b78b923 | 2017-11-17 16:12:37 +0900 | [diff] [blame] | 18 | decorators.skip_because from tempest.lib | 
| Ken'ichi Ohmichi | c0d96be | 2015-11-11 12:33:48 +0000 | [diff] [blame] | 19 | - [T110] Check that service client names of GET should be consistent | 
| Ken'ichi Ohmichi | 4f525f7 | 2016-03-25 15:20:01 -0700 | [diff] [blame] | 20 | - [T111] Check that service client names of DELETE should be consistent | 
| Ken'ichi Ohmichi | 0dc9747 | 2016-03-25 15:10:08 -0700 | [diff] [blame] | 21 | - [T112] Check that tempest.lib should not import local tempest code | 
| Ken'ichi Ohmichi | d079c89 | 2016-04-19 11:23:36 -0700 | [diff] [blame] | 22 | - [T113] Check that tests use data_utils.rand_uuid() instead of uuid.uuid4() | 
| Matthew Treinish | 59d9eaa | 2016-05-31 23:42:55 -0400 | [diff] [blame] | 23 | - [T114] Check that tempest.lib does not use tempest config | 
| Ken'ichi Ohmichi | f741d0b | 2017-05-01 16:56:14 -0700 | [diff] [blame] | 24 | - [T115] Check that admin tests should exist under admin path | 
| Ghanshyam | 2a180b8 | 2014-06-16 13:54:22 +0900 | [diff] [blame] | 25 | - [N322] Method's default argument shouldn't be mutable | 
| junboli | bc2ae86 | 2017-07-29 15:46:48 +0800 | [diff] [blame] | 26 | - [T116] Unsupported 'message' Exception attribute in PY3 | 
| Felipe Monteiro | 4d011af | 2018-07-18 00:11:48 -0400 | [diff] [blame] | 27 | - [T117] Check negative tests have ``@decorators.attr(type=['negative'])`` | 
|  | 28 | applied. | 
| Takashi Kajinami | 2a5ef1b | 2021-11-29 15:48:25 +0900 | [diff] [blame] | 29 | - [T118] LOG.warn is deprecated. Enforce use of LOG.warning. | 
| Attila Fazekas | 23fdf1d | 2013-06-09 16:35:23 +0200 | [diff] [blame] | 30 |  | 
| Attila Fazekas | c07edb5 | 2019-03-17 15:38:03 +0100 | [diff] [blame] | 31 | It is recommended to use ``tox -eautopep8`` before submitting a patch. | 
|  | 32 |  | 
| Matthew Treinish | 8b37289 | 2012-12-07 17:13:16 -0500 | [diff] [blame] | 33 | Test Data/Configuration | 
|  | 34 | ----------------------- | 
|  | 35 | - Assume nothing about existing test data | 
|  | 36 | - Tests should be self contained (provide their own data) | 
|  | 37 | - Clean up test data at the completion of each test | 
|  | 38 | - Use configuration files for values that will vary by environment | 
|  | 39 |  | 
| Felipe Monteiro | f22e6ec | 2018-11-03 17:51:18 -0400 | [diff] [blame] | 40 | Supported OpenStack Components | 
|  | 41 | ------------------------------ | 
|  | 42 |  | 
|  | 43 | Tempest's :ref:`library` and :ref:`plugin interface <tempest_plugin>` can be | 
|  | 44 | leveraged to support integration testing for virtually any OpenStack component. | 
|  | 45 |  | 
|  | 46 | However, Tempest only offers **in-tree** integration testing coverage for the | 
|  | 47 | following components: | 
|  | 48 |  | 
|  | 49 | * Cinder | 
|  | 50 | * Glance | 
|  | 51 | * Keystone | 
|  | 52 | * Neutron | 
|  | 53 | * Nova | 
|  | 54 | * Swift | 
|  | 55 |  | 
|  | 56 | Historically, Tempest offered in-tree testing for other components as well, but | 
|  | 57 | since the introduction of the `External Plugin Interface`_, Tempest's in-tree | 
|  | 58 | testing scope has been limited to the projects above. Integration tests for | 
|  | 59 | projects not included above should go into one of the | 
|  | 60 | `relevant plugin projects`_. | 
|  | 61 |  | 
|  | 62 | .. _External Plugin Interface: https://specs.openstack.org/openstack/qa-specs/specs/tempest/implemented/tempest-external-plugin-interface.html | 
| Ghanshyam Mann | da3bb61 | 2020-05-04 20:52:01 -0500 | [diff] [blame] | 63 | .. _relevant plugin projects: https://docs.openstack.org/tempest/latest/plugins/plugin-registry.html#detected-plugins | 
| Matthew Treinish | 8b37289 | 2012-12-07 17:13:16 -0500 | [diff] [blame] | 64 |  | 
| Attila Fazekas | 10fd63d | 2013-07-04 18:38:21 +0200 | [diff] [blame] | 65 | Exception Handling | 
|  | 66 | ------------------ | 
|  | 67 | According to the ``The Zen of Python`` the | 
| Attila Fazekas | 58d2330 | 2013-07-24 10:25:02 +0200 | [diff] [blame] | 68 | ``Errors should never pass silently.`` | 
| Attila Fazekas | 10fd63d | 2013-07-04 18:38:21 +0200 | [diff] [blame] | 69 | Tempest usually runs in special environment (jenkins gate jobs), in every | 
|  | 70 | error or failure situation we should provide as much error related | 
|  | 71 | information as possible, because we usually do not have the chance to | 
|  | 72 | investigate the situation after the issue happened. | 
|  | 73 |  | 
|  | 74 | In every test case the abnormal situations must be very verbosely explained, | 
|  | 75 | by the exception and the log. | 
|  | 76 |  | 
|  | 77 | In most cases the very first issue is the most important information. | 
|  | 78 |  | 
| Mithil Arun | be067ec | 2014-11-05 15:58:50 +0530 | [diff] [blame] | 79 | Try to avoid using ``try`` blocks in the test cases, as both the ``except`` | 
|  | 80 | and ``finally`` blocks could replace the original exception, | 
| Attila Fazekas | 10fd63d | 2013-07-04 18:38:21 +0200 | [diff] [blame] | 81 | when the additional operations leads to another exception. | 
|  | 82 |  | 
| Mithil Arun | be067ec | 2014-11-05 15:58:50 +0530 | [diff] [blame] | 83 | Just letting an exception to propagate, is not a bad idea in a test case, | 
| Bruce R. Montague | 44a6a19 | 2013-12-17 09:06:04 -0800 | [diff] [blame] | 84 | at all. | 
| Attila Fazekas | 10fd63d | 2013-07-04 18:38:21 +0200 | [diff] [blame] | 85 |  | 
|  | 86 | Try to avoid using any exception handling construct which can hide the errors | 
|  | 87 | origin. | 
|  | 88 |  | 
|  | 89 | If you really need to use a ``try`` block, please ensure the original | 
|  | 90 | exception at least logged.  When the exception is logged you usually need | 
|  | 91 | to ``raise`` the same or a different exception anyway. | 
|  | 92 |  | 
| Chris Yeoh | c2ff727 | 2013-07-22 22:25:25 +0930 | [diff] [blame] | 93 | Use of ``self.addCleanup`` is often a good way to avoid having to catch | 
|  | 94 | exceptions and still ensure resources are correctly cleaned up if the | 
|  | 95 | test fails part way through. | 
|  | 96 |  | 
| Mithil Arun | be067ec | 2014-11-05 15:58:50 +0530 | [diff] [blame] | 97 | Use the ``self.assert*`` methods provided by the unit test framework. | 
|  | 98 | This signals the failures early on. | 
| Attila Fazekas | 10fd63d | 2013-07-04 18:38:21 +0200 | [diff] [blame] | 99 |  | 
| Mithil Arun | be067ec | 2014-11-05 15:58:50 +0530 | [diff] [blame] | 100 | Avoid using the ``self.fail`` alone, its stack trace will signal | 
| Bruce R. Montague | 44a6a19 | 2013-12-17 09:06:04 -0800 | [diff] [blame] | 101 | the ``self.fail`` line as the origin of the error. | 
| Attila Fazekas | 10fd63d | 2013-07-04 18:38:21 +0200 | [diff] [blame] | 102 |  | 
|  | 103 | Avoid constructing complex boolean expressions for assertion. | 
| Attila Fazekas | 7899d31 | 2013-08-16 09:18:17 +0200 | [diff] [blame] | 104 | The ``self.assertTrue`` or ``self.assertFalse`` without a ``msg`` argument, | 
|  | 105 | will just tell you the single boolean value, and you will not know anything | 
|  | 106 | about the values used in the formula, the ``msg`` argument might be good enough | 
|  | 107 | for providing more information. | 
|  | 108 |  | 
|  | 109 | Most other assert method can include more information by default. | 
| Attila Fazekas | 10fd63d | 2013-07-04 18:38:21 +0200 | [diff] [blame] | 110 | For example ``self.assertIn`` can include the whole set. | 
|  | 111 |  | 
| Matthew Treinish | f45ba2e | 2015-08-24 15:05:01 -0400 | [diff] [blame] | 112 | It is recommended to use testtools `matcher`_ for the more tricky assertions. | 
|  | 113 | You can implement your own specific `matcher`_ as well. | 
| Attila Fazekas | 7899d31 | 2013-08-16 09:18:17 +0200 | [diff] [blame] | 114 |  | 
| davyyy | ac670dc | 2017-11-16 21:27:03 +0800 | [diff] [blame] | 115 | .. _matcher: https://testtools.readthedocs.org/en/latest/for-test-authors.html#matchers | 
| Attila Fazekas | 7899d31 | 2013-08-16 09:18:17 +0200 | [diff] [blame] | 116 |  | 
| Attila Fazekas | 10fd63d | 2013-07-04 18:38:21 +0200 | [diff] [blame] | 117 | If the test case fails you can see the related logs and the information | 
|  | 118 | carried by the exception (exception class, backtrack and exception info). | 
| Mithil Arun | be067ec | 2014-11-05 15:58:50 +0530 | [diff] [blame] | 119 | This and the service logs are your only guide to finding the root cause of flaky | 
|  | 120 | issues. | 
| Attila Fazekas | 10fd63d | 2013-07-04 18:38:21 +0200 | [diff] [blame] | 121 |  | 
| Attila Fazekas | 7899d31 | 2013-08-16 09:18:17 +0200 | [diff] [blame] | 122 | Test cases are independent | 
|  | 123 | -------------------------- | 
|  | 124 | Every ``test_method`` must be callable individually and MUST NOT depends on, | 
|  | 125 | any other ``test_method`` or ``test_method`` ordering. | 
|  | 126 |  | 
|  | 127 | Test cases MAY depend on commonly initialized resources/facilities, like | 
|  | 128 | credentials management, testresources and so on. These facilities, MUST be able | 
| Mithil Arun | be067ec | 2014-11-05 15:58:50 +0530 | [diff] [blame] | 129 | to work even if just one ``test_method`` is selected for execution. | 
| Attila Fazekas | 7899d31 | 2013-08-16 09:18:17 +0200 | [diff] [blame] | 130 |  | 
| Matthew Treinish | 5e4c0f2 | 2013-09-10 18:38:28 +0000 | [diff] [blame] | 131 | Service Tagging | 
|  | 132 | --------------- | 
|  | 133 | Service tagging is used to specify which services are exercised by a particular | 
| mmkmmk57 | ce3bb9b | 2017-09-20 13:41:41 +0900 | [diff] [blame] | 134 | test method. You specify the services with the ``tempest.common.utils.services`` | 
| Jordan Pittier | 74a56ab | 2017-04-26 16:46:20 +0200 | [diff] [blame] | 135 | decorator. For example: | 
| Matthew Treinish | 5e4c0f2 | 2013-09-10 18:38:28 +0000 | [diff] [blame] | 136 |  | 
| Felipe Monteiro | 46920b8 | 2018-07-09 23:58:20 -0400 | [diff] [blame] | 137 | ``@utils.services('compute', 'image')`` | 
| Matthew Treinish | 5e4c0f2 | 2013-09-10 18:38:28 +0000 | [diff] [blame] | 138 |  | 
|  | 139 | Valid service tag names are the same as the list of directories in tempest.api | 
|  | 140 | that have tests. | 
|  | 141 |  | 
| Jordan Pittier | 74a56ab | 2017-04-26 16:46:20 +0200 | [diff] [blame] | 142 | For scenario tests having a service tag is required. For the API tests service | 
|  | 143 | tags are only needed if the test method makes an API call (either directly or | 
| Matthew Treinish | 5e4c0f2 | 2013-09-10 18:38:28 +0000 | [diff] [blame] | 144 | indirectly through another service) that differs from the parent directory | 
| Jordan Pittier | 74a56ab | 2017-04-26 16:46:20 +0200 | [diff] [blame] | 145 | name. For example, any test that make an API call to a service other than Nova | 
|  | 146 | in ``tempest.api.compute`` would require a service tag for those services, | 
|  | 147 | however they do not need to be tagged as ``compute``. | 
| Matthew Treinish | 5e4c0f2 | 2013-09-10 18:38:28 +0000 | [diff] [blame] | 148 |  | 
| Felipe Monteiro | 46920b8 | 2018-07-09 23:58:20 -0400 | [diff] [blame] | 149 | Test Attributes | 
|  | 150 | --------------- | 
|  | 151 | Tempest leverages `test attributes`_ which are a simple but effective way of | 
|  | 152 | distinguishing between different "types" of API tests. A test can be "tagged" | 
|  | 153 | with such attributes using the ``decorators.attr`` decorator, for example:: | 
|  | 154 |  | 
|  | 155 | @decorators.attr(type=['negative']) | 
|  | 156 | def test_aggregate_create_aggregate_name_length_less_than_1(self): | 
|  | 157 | [...] | 
|  | 158 |  | 
|  | 159 | These test attributes can be used for test selection via regular expressions. | 
|  | 160 | For example, ``(?!.*\[.*\bslow\b.*\])(^tempest\.scenario)`` runs all the tests | 
|  | 161 | in the ``scenario`` test module, *except* for those tagged with the ``slow`` | 
|  | 162 | attribute (via a negative lookahead in the regular expression). These | 
|  | 163 | attributes are used in Tempest's ``tox.ini`` as well as Tempest's Zuul job | 
|  | 164 | definitions for specifying particular batches of Tempest test suites to run. | 
|  | 165 |  | 
|  | 166 | .. _test attributes: https://testtools.readthedocs.io/en/latest/for-test-authors.html?highlight=attr#test-attributes | 
|  | 167 |  | 
|  | 168 | Negative Attribute | 
|  | 169 | ^^^^^^^^^^^^^^^^^^ | 
|  | 170 | The ``type='negative'`` attribute is used to signify that a test is a negative | 
|  | 171 | test, which is a test that handles invalid input gracefully. This attribute | 
|  | 172 | should be applied to all negative test scenarios. | 
|  | 173 |  | 
|  | 174 | This attribute must be applied to each test that belongs to a negative test | 
|  | 175 | class, i.e. a test class name ending with "Negative.*" substring. | 
|  | 176 |  | 
| Felipe Monteiro | 46920b8 | 2018-07-09 23:58:20 -0400 | [diff] [blame] | 177 | Slow Attribute | 
|  | 178 | ^^^^^^^^^^^^^^ | 
|  | 179 | The ``type='slow'`` attribute is used to signify that a test takes a long time | 
|  | 180 | to run, relatively speaking. This attribute is usually applied to | 
|  | 181 | :ref:`scenario tests <scenario_field_guide>`, which involve a complicated | 
|  | 182 | series of API operations, the total runtime of which can be relatively long. | 
|  | 183 | This long runtime has performance implications on `Zuul`_ jobs, which is why | 
|  | 184 | the ``slow`` attribute is leveraged to run slow tests on a selective basis, | 
|  | 185 | to keep total `Zuul`_ job runtime down to a reasonable time frame. | 
|  | 186 |  | 
|  | 187 | .. _Zuul: https://docs.openstack.org/infra/zuul/ | 
|  | 188 |  | 
|  | 189 | Smoke Attribute | 
|  | 190 | ^^^^^^^^^^^^^^^ | 
|  | 191 | The ``type='smoke'`` attribute is used to signify that a test is a so-called | 
|  | 192 | smoke test, which is a type of test that tests the most vital OpenStack | 
|  | 193 | functionality, like listing servers or flavors or creating volumes. The | 
|  | 194 | attribute should be sparingly applied to only the tests that sanity-check the | 
|  | 195 | most essential functionality of an OpenStack cloud. | 
|  | 196 |  | 
| Andrea Frittoli | a5ddd55 | 2014-08-19 18:30:00 +0100 | [diff] [blame] | 197 | Test fixtures and resources | 
|  | 198 | --------------------------- | 
|  | 199 | Test level resources should be cleaned-up after the test execution. Clean-up | 
| Masayuki Igawa | bbbaad6 | 2017-11-21 16:04:03 +0900 | [diff] [blame] | 200 | is best scheduled using ``addCleanup`` which ensures that the resource cleanup | 
| Andrea Frittoli | a5ddd55 | 2014-08-19 18:30:00 +0100 | [diff] [blame] | 201 | code is always invoked, and in reverse order with respect to the creation | 
|  | 202 | order. | 
|  | 203 |  | 
| Masayuki Igawa | bbbaad6 | 2017-11-21 16:04:03 +0900 | [diff] [blame] | 204 | Test class level resources should be defined in the ``resource_setup`` method | 
|  | 205 | of the test class, except for any credential obtained from the credentials | 
|  | 206 | provider, which should be set-up in the ``setup_credentials`` method. | 
|  | 207 | Cleanup is best scheduled using ``addClassResourceCleanup`` which ensures that | 
| Andrea Frittoli | 3be5748 | 2017-08-25 22:41:26 +0100 | [diff] [blame] | 208 | the cleanup code is always invoked, and in reverse order with respect to the | 
|  | 209 | creation order. | 
|  | 210 |  | 
|  | 211 | In both cases - test level and class level cleanups - a wait loop should be | 
|  | 212 | scheduled before the actual delete of resources with an asynchronous delete. | 
| Andrea Frittoli | a5ddd55 | 2014-08-19 18:30:00 +0100 | [diff] [blame] | 213 |  | 
| Masayuki Igawa | bbbaad6 | 2017-11-21 16:04:03 +0900 | [diff] [blame] | 214 | The test base class ``BaseTestCase`` defines Tempest framework for class level | 
|  | 215 | fixtures. ``setUpClass`` and ``tearDownClass`` are defined here and cannot be | 
| Andrea Frittoli | a5ddd55 | 2014-08-19 18:30:00 +0100 | [diff] [blame] | 216 | overwritten by subclasses (enforced via hacking rule T105). | 
|  | 217 |  | 
|  | 218 | Set-up is split in a series of steps (setup stages), which can be overwritten | 
|  | 219 | by test classes. Set-up stages are: | 
| Masayuki Igawa | e63cf0f | 2016-05-25 10:25:21 +0900 | [diff] [blame] | 220 |  | 
| Masayuki Igawa | bbbaad6 | 2017-11-21 16:04:03 +0900 | [diff] [blame] | 221 | - ``skip_checks`` | 
|  | 222 | - ``setup_credentials`` | 
|  | 223 | - ``setup_clients`` | 
|  | 224 | - ``resource_setup`` | 
| Andrea Frittoli | a5ddd55 | 2014-08-19 18:30:00 +0100 | [diff] [blame] | 225 |  | 
|  | 226 | Tear-down is also split in a series of steps (teardown stages), which are | 
|  | 227 | stacked for execution only if the corresponding setup stage had been | 
|  | 228 | reached during the setup phase. Tear-down stages are: | 
| Masayuki Igawa | e63cf0f | 2016-05-25 10:25:21 +0900 | [diff] [blame] | 229 |  | 
| Masayuki Igawa | bbbaad6 | 2017-11-21 16:04:03 +0900 | [diff] [blame] | 230 | - ``clear_credentials`` (defined in the base test class) | 
|  | 231 | - ``resource_cleanup`` | 
| Andrea Frittoli | a5ddd55 | 2014-08-19 18:30:00 +0100 | [diff] [blame] | 232 |  | 
|  | 233 | Skipping Tests | 
|  | 234 | -------------- | 
|  | 235 | Skipping tests should be based on configuration only. If that is not possible, | 
|  | 236 | it is likely that either a configuration flag is missing, or the test should | 
|  | 237 | fail rather than be skipped. | 
|  | 238 | Using discovery for skipping tests is generally discouraged. | 
|  | 239 |  | 
|  | 240 | When running a test that requires a certain "feature" in the target | 
|  | 241 | cloud, if that feature is missing we should fail, because either the test | 
|  | 242 | configuration is invalid, or the cloud is broken and the expected "feature" is | 
|  | 243 | not there even if the cloud was configured with it. | 
|  | 244 |  | 
| Matthew Treinish | 8b79bb3 | 2013-10-10 17:11:05 -0400 | [diff] [blame] | 245 | Negative Tests | 
|  | 246 | -------------- | 
| Chris Hoge | 2b47841 | 2016-06-23 16:03:28 -0700 | [diff] [blame] | 247 | Error handling is an important aspect of API design and usage. Negative | 
|  | 248 | tests are a way to ensure that an application can gracefully handle | 
|  | 249 | invalid or unexpected input. However, as a black box integration test | 
|  | 250 | suite, Tempest is not suitable for handling all negative test cases, as | 
|  | 251 | the wide variety and complexity of negative tests can lead to long test | 
|  | 252 | runs and knowledge of internal implementation details. The bulk of | 
| Ken'ichi Ohmichi | 8db4075 | 2016-09-28 14:43:05 -0700 | [diff] [blame] | 253 | negative testing should be handled with project function tests. | 
|  | 254 | All negative tests should be based on `API-WG guideline`_ . Such negative | 
|  | 255 | tests can block any changes from accurate failure code to invalid one. | 
|  | 256 |  | 
| davyyy | ac670dc | 2017-11-16 21:27:03 +0800 | [diff] [blame] | 257 | .. _API-WG guideline: https://specs.openstack.org/openstack/api-wg/guidelines/http.html#failure-code-clarifications | 
| Ken'ichi Ohmichi | 8db4075 | 2016-09-28 14:43:05 -0700 | [diff] [blame] | 258 |  | 
|  | 259 | If facing some gray area which is not clarified on the above guideline, propose | 
|  | 260 | a new guideline to the API-WG. With a proposal to the API-WG we will be able to | 
|  | 261 | build a consensus across all OpenStack projects and improve the quality and | 
|  | 262 | consistency of all the APIs. | 
|  | 263 |  | 
|  | 264 | In addition, we have some guidelines for additional negative tests. | 
|  | 265 |  | 
|  | 266 | - About BadRequest(HTTP400) case: We can add a single negative tests of | 
|  | 267 | BadRequest for each resource and method(POST, PUT). | 
|  | 268 | Please don't implement more negative tests on the same combination of | 
|  | 269 | resource and method even if API request parameters are different from | 
|  | 270 | the existing test. | 
|  | 271 | - About NotFound(HTTP404) case: We can add a single negative tests of | 
|  | 272 | NotFound for each resource and method(GET, PUT, DELETE, HEAD). | 
|  | 273 | Please don't implement more negative tests on the same combination | 
|  | 274 | of resource and method. | 
|  | 275 |  | 
|  | 276 | The above guidelines don't cover all cases and we will grow these guidelines | 
|  | 277 | organically over time. Patches outside of the above guidelines are left up to | 
|  | 278 | the reviewers' discretion and if we face some conflicts between reviewers, we | 
|  | 279 | will expand the guideline based on our discussion and experience. | 
| Matthew Treinish | 8b79bb3 | 2013-10-10 17:11:05 -0400 | [diff] [blame] | 280 |  | 
| Giulio Fidente | 83181a9 | 2013-10-01 06:02:24 +0200 | [diff] [blame] | 281 | Test skips because of Known Bugs | 
|  | 282 | -------------------------------- | 
| Giulio Fidente | 83181a9 | 2013-10-01 06:02:24 +0200 | [diff] [blame] | 283 | If a test is broken because of a bug it is appropriate to skip the test until | 
| Jordan Pittier | 74a56ab | 2017-04-26 16:46:20 +0200 | [diff] [blame] | 284 | bug has been fixed. You should use the ``skip_because`` decorator so that | 
| Giulio Fidente | 83181a9 | 2013-10-01 06:02:24 +0200 | [diff] [blame] | 285 | Tempest's skip tracking tool can watch the bug status. | 
|  | 286 |  | 
|  | 287 | Example:: | 
|  | 288 |  | 
|  | 289 | @skip_because(bug="980688") | 
|  | 290 | def test_this_and_that(self): | 
|  | 291 | ... | 
|  | 292 |  | 
| Chris Yeoh | c2ff727 | 2013-07-22 22:25:25 +0930 | [diff] [blame] | 293 | Guidelines | 
|  | 294 | ---------- | 
|  | 295 | - Do not submit changesets with only testcases which are skipped as | 
|  | 296 | they will not be merged. | 
|  | 297 | - Consistently check the status code of responses in testcases. The | 
|  | 298 | earlier a problem is detected the easier it is to debug, especially | 
|  | 299 | where there is complicated setup required. | 
| Matthew Treinish | 96c28d1 | 2013-09-16 17:05:09 +0000 | [diff] [blame] | 300 |  | 
| DennyZhang | 900f02b | 2013-09-23 08:34:04 -0500 | [diff] [blame] | 301 | Parallel Test Execution | 
|  | 302 | ----------------------- | 
| Matthew Treinish | 96c28d1 | 2013-09-16 17:05:09 +0000 | [diff] [blame] | 303 | Tempest by default runs its tests in parallel this creates the possibility for | 
|  | 304 | interesting interactions between tests which can cause unexpected failures. | 
| Andrea Frittoli (andreaf) | 17209bb | 2015-05-22 10:16:57 -0700 | [diff] [blame] | 305 | Dynamic credentials provides protection from most of the potential race | 
|  | 306 | conditions between tests outside the same class. But there are still a few of | 
|  | 307 | things to watch out for to try to avoid issues when running your tests in | 
|  | 308 | parallel. | 
| Matthew Treinish | 96c28d1 | 2013-09-16 17:05:09 +0000 | [diff] [blame] | 309 |  | 
| Sean Dague | ed6e586 | 2016-04-04 10:49:13 -0400 | [diff] [blame] | 310 | - Resources outside of a project scope still have the potential to conflict. This | 
| Matthew Treinish | 96c28d1 | 2013-09-16 17:05:09 +0000 | [diff] [blame] | 311 | is a larger concern for the admin tests since most resources and actions that | 
| Sean Dague | ed6e586 | 2016-04-04 10:49:13 -0400 | [diff] [blame] | 312 | require admin privileges are outside of projects. | 
| Matthew Treinish | 96c28d1 | 2013-09-16 17:05:09 +0000 | [diff] [blame] | 313 |  | 
|  | 314 | - Races between methods in the same class are not a problem because | 
| Jordan Pittier | 74a56ab | 2017-04-26 16:46:20 +0200 | [diff] [blame] | 315 | parallelization in Tempest is at the test class level, but if there is a json | 
| Matthew Treinish | 96c28d1 | 2013-09-16 17:05:09 +0000 | [diff] [blame] | 316 | and xml version of the same test class there could still be a race between | 
|  | 317 | methods. | 
|  | 318 |  | 
| jeremy.zhang | c0f9556 | 2017-05-26 13:41:57 +0800 | [diff] [blame] | 319 | - The rand_name() function from tempest.lib.common.utils.data_utils should be | 
|  | 320 | used anywhere a resource is created with a name. Static naming should be | 
|  | 321 | avoided to prevent resource conflicts. | 
| Matthew Treinish | 96c28d1 | 2013-09-16 17:05:09 +0000 | [diff] [blame] | 322 |  | 
|  | 323 | - If the execution of a set of tests is required to be serialized then locking | 
| Jordan Pittier | 74a56ab | 2017-04-26 16:46:20 +0200 | [diff] [blame] | 324 | can be used to perform this. See usage of ``LockFixture`` for examples of | 
|  | 325 | using locking. | 
| Marc Koderer | 31fe483 | 2013-11-06 17:02:03 +0100 | [diff] [blame] | 326 |  | 
| Matthew Treinish | 6eb0585 | 2013-11-26 15:28:12 +0000 | [diff] [blame] | 327 | Sample Configuration File | 
|  | 328 | ------------------------- | 
|  | 329 | The sample config file is autogenerated using a script. If any changes are made | 
| David Kranz | fb0f51f | 2014-11-11 14:07:20 -0500 | [diff] [blame] | 330 | to the config variables in tempest/config.py then the sample config file must be | 
|  | 331 | regenerated. This can be done running:: | 
|  | 332 |  | 
| Hai Shi | 6f52fc5 | 2017-04-03 21:17:37 +0800 | [diff] [blame] | 333 | tox -e genconfig | 
| Matthew Treinish | ecf212c | 2013-12-06 18:23:54 +0000 | [diff] [blame] | 334 |  | 
|  | 335 | Unit Tests | 
|  | 336 | ---------- | 
| Jordan Pittier | 74a56ab | 2017-04-26 16:46:20 +0200 | [diff] [blame] | 337 | Unit tests are a separate class of tests in Tempest. They verify Tempest | 
| Matthew Treinish | ecf212c | 2013-12-06 18:23:54 +0000 | [diff] [blame] | 338 | itself, and thus have a different set of guidelines around them: | 
|  | 339 |  | 
|  | 340 | 1. They can not require anything running externally. All you should need to | 
|  | 341 | run the unit tests is the git tree, python and the dependencies installed. | 
|  | 342 | This includes running services, a config file, etc. | 
|  | 343 |  | 
|  | 344 | 2. The unit tests cannot use setUpClass, instead fixtures and testresources | 
|  | 345 | should be used for shared state between tests. | 
| Matthew Treinish | 5507888 | 2014-08-12 19:01:34 -0400 | [diff] [blame] | 346 |  | 
|  | 347 |  | 
|  | 348 | .. _TestDocumentation: | 
|  | 349 |  | 
|  | 350 | Test Documentation | 
|  | 351 | ------------------ | 
|  | 352 | For tests being added we need to require inline documentation in the form of | 
| Xicheng Chang | 6fb98ec | 2015-08-13 14:02:52 -0700 | [diff] [blame] | 353 | docstrings to explain what is being tested. In API tests for a new API a class | 
| Matthew Treinish | 5507888 | 2014-08-12 19:01:34 -0400 | [diff] [blame] | 354 | level docstring should be added to an API reference doc. If one doesn't exist | 
|  | 355 | a TODO comment should be put indicating that the reference needs to be added. | 
|  | 356 | For individual API test cases a method level docstring should be used to | 
|  | 357 | explain the functionality being tested if the test name isn't descriptive | 
|  | 358 | enough. For example:: | 
|  | 359 |  | 
|  | 360 | def test_get_role_by_id(self): | 
|  | 361 | """Get a role by its id.""" | 
|  | 362 |  | 
|  | 363 | the docstring there is superfluous and shouldn't be added. but for a method | 
|  | 364 | like:: | 
|  | 365 |  | 
|  | 366 | def test_volume_backup_create_get_detailed_list_restore_delete(self): | 
|  | 367 | pass | 
|  | 368 |  | 
|  | 369 | a docstring would be useful because while the test title is fairly descriptive | 
|  | 370 | the operations being performed are complex enough that a bit more explanation | 
|  | 371 | will help people figure out the intent of the test. | 
|  | 372 |  | 
|  | 373 | For scenario tests a class level docstring describing the steps in the scenario | 
|  | 374 | is required. If there is more than one test case in the class individual | 
|  | 375 | docstrings for the workflow in each test methods can be used instead. A good | 
|  | 376 | example of this would be:: | 
|  | 377 |  | 
| zhufl | 42bcb55 | 2018-09-17 16:06:30 +0800 | [diff] [blame] | 378 | class TestServerBasicOps(manager.ScenarioTest): | 
| Matthew Treinish | 5507888 | 2014-08-12 19:01:34 -0400 | [diff] [blame] | 379 |  | 
| zhufl | 42bcb55 | 2018-09-17 16:06:30 +0800 | [diff] [blame] | 380 | """The test suite for server basic operations | 
|  | 381 |  | 
|  | 382 | This smoke test case follows this basic set of operations: | 
|  | 383 | * Create a keypair for use in launching an instance | 
|  | 384 | * Create a security group to control network access in instance | 
|  | 385 | * Add simple permissive rules to the security group | 
|  | 386 | * Launch an instance | 
|  | 387 | * Perform ssh to instance | 
|  | 388 | * Verify metadata service | 
|  | 389 | * Verify metadata on config_drive | 
|  | 390 | * Terminate the instance | 
| Dougal Matthews | 4bebca0 | 2014-10-28 08:36:04 +0000 | [diff] [blame] | 391 | """ | 
| Matthew Treinish | a970d65 | 2015-03-11 15:39:24 -0400 | [diff] [blame] | 392 |  | 
| Chris Hoge | 0e000ed | 2015-07-28 14:19:53 -0500 | [diff] [blame] | 393 | Test Identification with Idempotent ID | 
|  | 394 | -------------------------------------- | 
|  | 395 |  | 
|  | 396 | Every function that provides a test must have an ``idempotent_id`` decorator | 
|  | 397 | that is a unique ``uuid-4`` instance. This ID is used to complement the fully | 
| Naomichi Wakui | dbe9aab | 2015-08-26 03:36:02 +0000 | [diff] [blame] | 398 | qualified test name and track test functionality through refactoring. The | 
| Chris Hoge | 0e000ed | 2015-07-28 14:19:53 -0500 | [diff] [blame] | 399 | format of the metadata looks like:: | 
|  | 400 |  | 
| Ken'ichi Ohmichi | 8a08211 | 2017-03-06 16:03:17 -0800 | [diff] [blame] | 401 | @decorators.idempotent_id('585e934c-448e-43c4-acbf-d06a9b899997') | 
| Chris Hoge | 0e000ed | 2015-07-28 14:19:53 -0500 | [diff] [blame] | 402 | def test_list_servers_with_detail(self): | 
|  | 403 | # The created server should be in the detailed list of all servers | 
|  | 404 | ... | 
|  | 405 |  | 
| Andrea Frittoli (andreaf) | 1370baf | 2016-04-29 14:26:22 -0500 | [diff] [blame] | 406 | Tempest.lib includes a ``check-uuid`` tool that will test for the existence | 
| Matthew Treinish | c1802bc | 2015-12-03 18:48:11 -0500 | [diff] [blame] | 407 | and uniqueness of idempotent_id metadata for every test. If you have | 
| Jordan Pittier | 74a56ab | 2017-04-26 16:46:20 +0200 | [diff] [blame] | 408 | Tempest installed you run the tool against Tempest by calling from the | 
|  | 409 | Tempest repo:: | 
| Chris Hoge | 0e000ed | 2015-07-28 14:19:53 -0500 | [diff] [blame] | 410 |  | 
| Matthew Treinish | c1802bc | 2015-12-03 18:48:11 -0500 | [diff] [blame] | 411 | check-uuid | 
| Chris Hoge | 0e000ed | 2015-07-28 14:19:53 -0500 | [diff] [blame] | 412 |  | 
|  | 413 | It can be invoked against any test suite by passing a package name:: | 
|  | 414 |  | 
| Matthew Treinish | c1802bc | 2015-12-03 18:48:11 -0500 | [diff] [blame] | 415 | check-uuid --package <package_name> | 
| Chris Hoge | 0e000ed | 2015-07-28 14:19:53 -0500 | [diff] [blame] | 416 |  | 
|  | 417 | Tests without an ``idempotent_id`` can be automatically fixed by running | 
|  | 418 | the command with the ``--fix`` flag, which will modify the source package | 
|  | 419 | by inserting randomly generated uuids for every test that does not have | 
|  | 420 | one:: | 
|  | 421 |  | 
| Matthew Treinish | c1802bc | 2015-12-03 18:48:11 -0500 | [diff] [blame] | 422 | check-uuid --fix | 
| Chris Hoge | 0e000ed | 2015-07-28 14:19:53 -0500 | [diff] [blame] | 423 |  | 
| Jordan Pittier | 74a56ab | 2017-04-26 16:46:20 +0200 | [diff] [blame] | 424 | The ``check-uuid`` tool is used as part of the Tempest gate job | 
| Chris Hoge | 0e000ed | 2015-07-28 14:19:53 -0500 | [diff] [blame] | 425 | to ensure that all tests have an ``idempotent_id`` decorator. | 
|  | 426 |  | 
| Matthew Treinish | a970d65 | 2015-03-11 15:39:24 -0400 | [diff] [blame] | 427 | Branchless Tempest Considerations | 
|  | 428 | --------------------------------- | 
|  | 429 |  | 
|  | 430 | Starting with the OpenStack Icehouse release Tempest no longer has any stable | 
|  | 431 | branches. This is to better ensure API consistency between releases because | 
|  | 432 | the API behavior should not change between releases. This means that the stable | 
|  | 433 | branches are also gated by the Tempest master branch, which also means that | 
|  | 434 | proposed commits to Tempest must work against both the master and all the | 
|  | 435 | currently supported stable branches of the projects. As such there are a few | 
|  | 436 | special considerations that have to be accounted for when pushing new changes | 
| Jordan Pittier | 74a56ab | 2017-04-26 16:46:20 +0200 | [diff] [blame] | 437 | to Tempest. | 
| Matthew Treinish | a970d65 | 2015-03-11 15:39:24 -0400 | [diff] [blame] | 438 |  | 
|  | 439 | 1. New Tests for new features | 
|  | 440 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | 
|  | 441 |  | 
|  | 442 | When adding tests for new features that were not in previous releases of the | 
| Felipe Monteiro | 356f059 | 2018-03-26 21:51:52 -0400 | [diff] [blame] | 443 | projects the new test has to be properly skipped with a feature flag. This can | 
|  | 444 | be just as simple as using the ``@utils.requires_ext()`` or | 
|  | 445 | ``testtools.skipUnless`` decorators to check if the required extension (or | 
|  | 446 | discoverable optional API) or feature is enabled or can be as difficult as | 
| Andrea Frittoli | cd36841 | 2017-08-14 21:37:56 +0100 | [diff] [blame] | 447 | adding a new config option to the appropriate section. If there isn't a method | 
|  | 448 | of selecting the new **feature** from the config file then there won't be a | 
| Felipe Monteiro | 356f059 | 2018-03-26 21:51:52 -0400 | [diff] [blame] | 449 | mechanism to disable the test with older stable releases and the new test | 
|  | 450 | won't be able to merge. | 
|  | 451 |  | 
|  | 452 | Introduction of a new feature flag requires specifying a default value for | 
|  | 453 | the corresponding config option that is appropriate in the latest OpenStack | 
|  | 454 | release. Because Tempest is branchless, the feature flag's default value will | 
|  | 455 | need to be overridden to a value that is appropriate in earlier releases | 
|  | 456 | in which the feature isn't available. In DevStack, this can be accomplished | 
|  | 457 | by modifying Tempest's `lib installation script`_ for previous branches | 
|  | 458 | (because DevStack is branched). | 
|  | 459 |  | 
| caoyuan | 349ba75 | 2019-04-23 19:40:06 +0800 | [diff] [blame] | 460 | .. _lib installation script: https://opendev.org/openstack/devstack/src/branch/master/lib/tempest | 
| Matthew Treinish | a970d65 | 2015-03-11 15:39:24 -0400 | [diff] [blame] | 461 |  | 
|  | 462 | 2. Bug fix on core project needing Tempest changes | 
|  | 463 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | 
|  | 464 |  | 
|  | 465 | When trying to land a bug fix which changes a tested API you'll have to use the | 
|  | 466 | following procedure:: | 
|  | 467 |  | 
| Jordan Pittier | 74a56ab | 2017-04-26 16:46:20 +0200 | [diff] [blame] | 468 | 1. Propose change to the project, get a +2 on the change even with failing | 
|  | 469 | 2. Propose skip on Tempest which will only be approved after the | 
| Matthew Treinish | a970d65 | 2015-03-11 15:39:24 -0400 | [diff] [blame] | 470 | corresponding change in the project has a +2 on change | 
| Jordan Pittier | 74a56ab | 2017-04-26 16:46:20 +0200 | [diff] [blame] | 471 | 3. Land project change in master and all open stable branches (if required) | 
|  | 472 | 4. Land changed test in Tempest | 
| Matthew Treinish | a970d65 | 2015-03-11 15:39:24 -0400 | [diff] [blame] | 473 |  | 
|  | 474 | Otherwise the bug fix won't be able to land in the project. | 
|  | 475 |  | 
| gaofei | 6ec582f | 2018-01-24 14:08:36 +0800 | [diff] [blame] | 476 | Handily, `Zuul's cross-repository dependencies | 
| junboli | 477fd02 | 2017-09-06 17:25:11 +0800 | [diff] [blame] | 477 | <https://docs.openstack.org/infra/zuul/user/gating.html#cross-project-dependencies>`_. | 
| Jordan Pittier | 74a56ab | 2017-04-26 16:46:20 +0200 | [diff] [blame] | 478 | can be leveraged to do without step 2 and to have steps 3 and 4 happen | 
|  | 479 | "atomically". To do that, make the patch written in step 1 to depend (refer to | 
|  | 480 | Zuul's documentation above) on the patch written in step 4. The commit message | 
|  | 481 | for the Tempest change should have a link to the Gerrit review that justifies | 
|  | 482 | that change. | 
|  | 483 |  | 
| Matthew Treinish | a970d65 | 2015-03-11 15:39:24 -0400 | [diff] [blame] | 484 | 3. New Tests for existing features | 
|  | 485 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | 
|  | 486 |  | 
|  | 487 | If a test is being added for a feature that exists in all the current releases | 
|  | 488 | of the projects then the only concern is that the API behavior is the same | 
|  | 489 | across all the versions of the project being tested. If the behavior is not | 
|  | 490 | consistent the test will not be able to merge. | 
|  | 491 |  | 
|  | 492 | API Stability | 
|  | 493 | ------------- | 
|  | 494 |  | 
|  | 495 | For new tests being added to Tempest the assumption is that the API being | 
|  | 496 | tested is considered stable and adheres to the OpenStack API stability | 
|  | 497 | guidelines. If an API is still considered experimental or in development then | 
|  | 498 | it should not be tested by Tempest until it is considered stable. |