ce3bb9ba1f
Documentation update. Change-Id: I3bca9de9a2e3fbc12647a1c55be0efc4434df2c0
411 lines
18 KiB
ReStructuredText
411 lines
18 KiB
ReStructuredText
Tempest Coding Guide
|
||
====================
|
||
|
||
- Step 1: Read the OpenStack Style Commandments
|
||
https://docs.openstack.org/hacking/latest/
|
||
- Step 2: Read on
|
||
|
||
Tempest Specific Commandments
|
||
------------------------------
|
||
|
||
- [T102] Cannot import OpenStack python clients in tempest/api &
|
||
tempest/scenario tests
|
||
- [T104] Scenario tests require a services decorator
|
||
- [T105] Tests cannot use setUpClass/tearDownClass
|
||
- [T106] vim configuration should not be kept in source files.
|
||
- [T107] Check that a service tag isn't in the module path
|
||
- [T108] Check no hyphen at the end of rand_name() argument
|
||
- [T109] Cannot use testtools.skip decorator; instead use
|
||
decorators.skip_because from tempest.lib
|
||
- [T110] Check that service client names of GET should be consistent
|
||
- [T111] Check that service client names of DELETE should be consistent
|
||
- [T112] Check that tempest.lib should not import local tempest code
|
||
- [T113] Check that tests use data_utils.rand_uuid() instead of uuid.uuid4()
|
||
- [T114] Check that tempest.lib does not use tempest config
|
||
- [T115] Check that admin tests should exist under admin path
|
||
- [N322] Method's default argument shouldn't be mutable
|
||
- [T116] Unsupported 'message' Exception attribute in PY3
|
||
|
||
Test Data/Configuration
|
||
-----------------------
|
||
- Assume nothing about existing test data
|
||
- Tests should be self contained (provide their own data)
|
||
- Clean up test data at the completion of each test
|
||
- Use configuration files for values that will vary by environment
|
||
|
||
|
||
Exception Handling
|
||
------------------
|
||
According to the ``The Zen of Python`` the
|
||
``Errors should never pass silently.``
|
||
Tempest usually runs in special environment (jenkins gate jobs), in every
|
||
error or failure situation we should provide as much error related
|
||
information as possible, because we usually do not have the chance to
|
||
investigate the situation after the issue happened.
|
||
|
||
In every test case the abnormal situations must be very verbosely explained,
|
||
by the exception and the log.
|
||
|
||
In most cases the very first issue is the most important information.
|
||
|
||
Try to avoid using ``try`` blocks in the test cases, as both the ``except``
|
||
and ``finally`` blocks could replace the original exception,
|
||
when the additional operations leads to another exception.
|
||
|
||
Just letting an exception to propagate, is not a bad idea in a test case,
|
||
at all.
|
||
|
||
Try to avoid using any exception handling construct which can hide the errors
|
||
origin.
|
||
|
||
If you really need to use a ``try`` block, please ensure the original
|
||
exception at least logged. When the exception is logged you usually need
|
||
to ``raise`` the same or a different exception anyway.
|
||
|
||
Use of ``self.addCleanup`` is often a good way to avoid having to catch
|
||
exceptions and still ensure resources are correctly cleaned up if the
|
||
test fails part way through.
|
||
|
||
Use the ``self.assert*`` methods provided by the unit test framework.
|
||
This signals the failures early on.
|
||
|
||
Avoid using the ``self.fail`` alone, its stack trace will signal
|
||
the ``self.fail`` line as the origin of the error.
|
||
|
||
Avoid constructing complex boolean expressions for assertion.
|
||
The ``self.assertTrue`` or ``self.assertFalse`` without a ``msg`` argument,
|
||
will just tell you the single boolean value, and you will not know anything
|
||
about the values used in the formula, the ``msg`` argument might be good enough
|
||
for providing more information.
|
||
|
||
Most other assert method can include more information by default.
|
||
For example ``self.assertIn`` can include the whole set.
|
||
|
||
It is recommended to use testtools `matcher`_ for the more tricky assertions.
|
||
You can implement your own specific `matcher`_ as well.
|
||
|
||
.. _matcher: http://testtools.readthedocs.org/en/latest/for-test-authors.html#matchers
|
||
|
||
If the test case fails you can see the related logs and the information
|
||
carried by the exception (exception class, backtrack and exception info).
|
||
This and the service logs are your only guide to finding the root cause of flaky
|
||
issues.
|
||
|
||
Test cases are independent
|
||
--------------------------
|
||
Every ``test_method`` must be callable individually and MUST NOT depends on,
|
||
any other ``test_method`` or ``test_method`` ordering.
|
||
|
||
Test cases MAY depend on commonly initialized resources/facilities, like
|
||
credentials management, testresources and so on. These facilities, MUST be able
|
||
to work even if just one ``test_method`` is selected for execution.
|
||
|
||
Service Tagging
|
||
---------------
|
||
Service tagging is used to specify which services are exercised by a particular
|
||
test method. You specify the services with the ``tempest.common.utils.services``
|
||
decorator. For example:
|
||
|
||
@utils.services('compute', 'image')
|
||
|
||
Valid service tag names are the same as the list of directories in tempest.api
|
||
that have tests.
|
||
|
||
For scenario tests having a service tag is required. For the API tests service
|
||
tags are only needed if the test method makes an API call (either directly or
|
||
indirectly through another service) that differs from the parent directory
|
||
name. For example, any test that make an API call to a service other than Nova
|
||
in ``tempest.api.compute`` would require a service tag for those services,
|
||
however they do not need to be tagged as ``compute``.
|
||
|
||
Test fixtures and resources
|
||
---------------------------
|
||
Test level resources should be cleaned-up after the test execution. Clean-up
|
||
is best scheduled using `addCleanup` which ensures that the resource cleanup
|
||
code is always invoked, and in reverse order with respect to the creation
|
||
order.
|
||
|
||
Test class level resources should be defined in the `resource_setup` method of
|
||
the test class, except for any credential obtained from the credentials
|
||
provider, which should be set-up in the `setup_credentials` method.
|
||
Cleanup is best scheduled using `addClassResourceCleanup` which ensures that
|
||
the cleanup code is always invoked, and in reverse order with respect to the
|
||
creation order.
|
||
|
||
In both cases - test level and class level cleanups - a wait loop should be
|
||
scheduled before the actual delete of resources with an asynchronous delete.
|
||
|
||
The test base class `BaseTestCase` defines Tempest framework for class level
|
||
fixtures. `setUpClass` and `tearDownClass` are defined here and cannot be
|
||
overwritten by subclasses (enforced via hacking rule T105).
|
||
|
||
Set-up is split in a series of steps (setup stages), which can be overwritten
|
||
by test classes. Set-up stages are:
|
||
|
||
- `skip_checks`
|
||
- `setup_credentials`
|
||
- `setup_clients`
|
||
- `resource_setup`
|
||
|
||
Tear-down is also split in a series of steps (teardown stages), which are
|
||
stacked for execution only if the corresponding setup stage had been
|
||
reached during the setup phase. Tear-down stages are:
|
||
|
||
- `clear_credentials` (defined in the base test class)
|
||
- `resource_cleanup`
|
||
|
||
Skipping Tests
|
||
--------------
|
||
Skipping tests should be based on configuration only. If that is not possible,
|
||
it is likely that either a configuration flag is missing, or the test should
|
||
fail rather than be skipped.
|
||
Using discovery for skipping tests is generally discouraged.
|
||
|
||
When running a test that requires a certain "feature" in the target
|
||
cloud, if that feature is missing we should fail, because either the test
|
||
configuration is invalid, or the cloud is broken and the expected "feature" is
|
||
not there even if the cloud was configured with it.
|
||
|
||
Negative Tests
|
||
--------------
|
||
Error handling is an important aspect of API design and usage. Negative
|
||
tests are a way to ensure that an application can gracefully handle
|
||
invalid or unexpected input. However, as a black box integration test
|
||
suite, Tempest is not suitable for handling all negative test cases, as
|
||
the wide variety and complexity of negative tests can lead to long test
|
||
runs and knowledge of internal implementation details. The bulk of
|
||
negative testing should be handled with project function tests.
|
||
All negative tests should be based on `API-WG guideline`_ . Such negative
|
||
tests can block any changes from accurate failure code to invalid one.
|
||
|
||
.. _API-WG guideline: http://specs.openstack.org/openstack/api-wg/guidelines/http.html#failure-code-clarifications
|
||
|
||
If facing some gray area which is not clarified on the above guideline, propose
|
||
a new guideline to the API-WG. With a proposal to the API-WG we will be able to
|
||
build a consensus across all OpenStack projects and improve the quality and
|
||
consistency of all the APIs.
|
||
|
||
In addition, we have some guidelines for additional negative tests.
|
||
|
||
- About BadRequest(HTTP400) case: We can add a single negative tests of
|
||
BadRequest for each resource and method(POST, PUT).
|
||
Please don't implement more negative tests on the same combination of
|
||
resource and method even if API request parameters are different from
|
||
the existing test.
|
||
- About NotFound(HTTP404) case: We can add a single negative tests of
|
||
NotFound for each resource and method(GET, PUT, DELETE, HEAD).
|
||
Please don't implement more negative tests on the same combination
|
||
of resource and method.
|
||
|
||
The above guidelines don't cover all cases and we will grow these guidelines
|
||
organically over time. Patches outside of the above guidelines are left up to
|
||
the reviewers' discretion and if we face some conflicts between reviewers, we
|
||
will expand the guideline based on our discussion and experience.
|
||
|
||
Test skips because of Known Bugs
|
||
--------------------------------
|
||
If a test is broken because of a bug it is appropriate to skip the test until
|
||
bug has been fixed. You should use the ``skip_because`` decorator so that
|
||
Tempest's skip tracking tool can watch the bug status.
|
||
|
||
Example::
|
||
|
||
@skip_because(bug="980688")
|
||
def test_this_and_that(self):
|
||
...
|
||
|
||
Guidelines
|
||
----------
|
||
- Do not submit changesets with only testcases which are skipped as
|
||
they will not be merged.
|
||
- Consistently check the status code of responses in testcases. The
|
||
earlier a problem is detected the easier it is to debug, especially
|
||
where there is complicated setup required.
|
||
|
||
Parallel Test Execution
|
||
-----------------------
|
||
Tempest by default runs its tests in parallel this creates the possibility for
|
||
interesting interactions between tests which can cause unexpected failures.
|
||
Dynamic credentials provides protection from most of the potential race
|
||
conditions between tests outside the same class. But there are still a few of
|
||
things to watch out for to try to avoid issues when running your tests in
|
||
parallel.
|
||
|
||
- Resources outside of a project scope still have the potential to conflict. This
|
||
is a larger concern for the admin tests since most resources and actions that
|
||
require admin privileges are outside of projects.
|
||
|
||
- Races between methods in the same class are not a problem because
|
||
parallelization in Tempest is at the test class level, but if there is a json
|
||
and xml version of the same test class there could still be a race between
|
||
methods.
|
||
|
||
- The rand_name() function from tempest.lib.common.utils.data_utils should be
|
||
used anywhere a resource is created with a name. Static naming should be
|
||
avoided to prevent resource conflicts.
|
||
|
||
- If the execution of a set of tests is required to be serialized then locking
|
||
can be used to perform this. See usage of ``LockFixture`` for examples of
|
||
using locking.
|
||
|
||
Sample Configuration File
|
||
-------------------------
|
||
The sample config file is autogenerated using a script. If any changes are made
|
||
to the config variables in tempest/config.py then the sample config file must be
|
||
regenerated. This can be done running::
|
||
|
||
tox -e genconfig
|
||
|
||
Unit Tests
|
||
----------
|
||
Unit tests are a separate class of tests in Tempest. They verify Tempest
|
||
itself, and thus have a different set of guidelines around them:
|
||
|
||
1. They can not require anything running externally. All you should need to
|
||
run the unit tests is the git tree, python and the dependencies installed.
|
||
This includes running services, a config file, etc.
|
||
|
||
2. The unit tests cannot use setUpClass, instead fixtures and testresources
|
||
should be used for shared state between tests.
|
||
|
||
|
||
.. _TestDocumentation:
|
||
|
||
Test Documentation
|
||
------------------
|
||
For tests being added we need to require inline documentation in the form of
|
||
docstrings to explain what is being tested. In API tests for a new API a class
|
||
level docstring should be added to an API reference doc. If one doesn't exist
|
||
a TODO comment should be put indicating that the reference needs to be added.
|
||
For individual API test cases a method level docstring should be used to
|
||
explain the functionality being tested if the test name isn't descriptive
|
||
enough. For example::
|
||
|
||
def test_get_role_by_id(self):
|
||
"""Get a role by its id."""
|
||
|
||
the docstring there is superfluous and shouldn't be added. but for a method
|
||
like::
|
||
|
||
def test_volume_backup_create_get_detailed_list_restore_delete(self):
|
||
pass
|
||
|
||
a docstring would be useful because while the test title is fairly descriptive
|
||
the operations being performed are complex enough that a bit more explanation
|
||
will help people figure out the intent of the test.
|
||
|
||
For scenario tests a class level docstring describing the steps in the scenario
|
||
is required. If there is more than one test case in the class individual
|
||
docstrings for the workflow in each test methods can be used instead. A good
|
||
example of this would be::
|
||
|
||
class TestVolumeBootPattern(manager.ScenarioTest):
|
||
"""
|
||
This test case attempts to reproduce the following steps:
|
||
|
||
* Create in Cinder some bootable volume importing a Glance image
|
||
* Boot an instance from the bootable volume
|
||
* Write content to the volume
|
||
* Delete an instance and Boot a new instance from the volume
|
||
* Check written content in the instance
|
||
* Create a volume snapshot while the instance is running
|
||
* Boot an additional instance from the new snapshot based volume
|
||
* Check written content in the instance booted from snapshot
|
||
"""
|
||
|
||
Test Identification with Idempotent ID
|
||
--------------------------------------
|
||
|
||
Every function that provides a test must have an ``idempotent_id`` decorator
|
||
that is a unique ``uuid-4`` instance. This ID is used to complement the fully
|
||
qualified test name and track test functionality through refactoring. The
|
||
format of the metadata looks like::
|
||
|
||
@decorators.idempotent_id('585e934c-448e-43c4-acbf-d06a9b899997')
|
||
def test_list_servers_with_detail(self):
|
||
# The created server should be in the detailed list of all servers
|
||
...
|
||
|
||
Tempest.lib includes a ``check-uuid`` tool that will test for the existence
|
||
and uniqueness of idempotent_id metadata for every test. If you have
|
||
Tempest installed you run the tool against Tempest by calling from the
|
||
Tempest repo::
|
||
|
||
check-uuid
|
||
|
||
It can be invoked against any test suite by passing a package name::
|
||
|
||
check-uuid --package <package_name>
|
||
|
||
Tests without an ``idempotent_id`` can be automatically fixed by running
|
||
the command with the ``--fix`` flag, which will modify the source package
|
||
by inserting randomly generated uuids for every test that does not have
|
||
one::
|
||
|
||
check-uuid --fix
|
||
|
||
The ``check-uuid`` tool is used as part of the Tempest gate job
|
||
to ensure that all tests have an ``idempotent_id`` decorator.
|
||
|
||
Branchless Tempest Considerations
|
||
---------------------------------
|
||
|
||
Starting with the OpenStack Icehouse release Tempest no longer has any stable
|
||
branches. This is to better ensure API consistency between releases because
|
||
the API behavior should not change between releases. This means that the stable
|
||
branches are also gated by the Tempest master branch, which also means that
|
||
proposed commits to Tempest must work against both the master and all the
|
||
currently supported stable branches of the projects. As such there are a few
|
||
special considerations that have to be accounted for when pushing new changes
|
||
to Tempest.
|
||
|
||
1. New Tests for new features
|
||
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||
|
||
When adding tests for new features that were not in previous releases of the
|
||
projects the new test has to be properly skipped with a feature flag. Whether
|
||
this is just as simple as using the @utils.requires_ext() decorator to
|
||
check if the required extension (or discoverable optional API) is enabled or
|
||
adding a new config option to the appropriate section. If there isn't a method
|
||
of selecting the new **feature** from the config file then there won't be a
|
||
mechanism to disable the test with older stable releases and the new test won't
|
||
be able to merge.
|
||
|
||
2. Bug fix on core project needing Tempest changes
|
||
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||
|
||
When trying to land a bug fix which changes a tested API you'll have to use the
|
||
following procedure::
|
||
|
||
1. Propose change to the project, get a +2 on the change even with failing
|
||
2. Propose skip on Tempest which will only be approved after the
|
||
corresponding change in the project has a +2 on change
|
||
3. Land project change in master and all open stable branches (if required)
|
||
4. Land changed test in Tempest
|
||
|
||
Otherwise the bug fix won't be able to land in the project.
|
||
|
||
Handily, `Zuul’s cross-repository dependencies
|
||
<https://docs.openstack.org/infra/zuul/user/gating.html#cross-project-dependencies>`_.
|
||
can be leveraged to do without step 2 and to have steps 3 and 4 happen
|
||
"atomically". To do that, make the patch written in step 1 to depend (refer to
|
||
Zuul's documentation above) on the patch written in step 4. The commit message
|
||
for the Tempest change should have a link to the Gerrit review that justifies
|
||
that change.
|
||
|
||
3. New Tests for existing features
|
||
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||
|
||
If a test is being added for a feature that exists in all the current releases
|
||
of the projects then the only concern is that the API behavior is the same
|
||
across all the versions of the project being tested. If the behavior is not
|
||
consistent the test will not be able to merge.
|
||
|
||
API Stability
|
||
-------------
|
||
|
||
For new tests being added to Tempest the assumption is that the API being
|
||
tested is considered stable and adheres to the OpenStack API stability
|
||
guidelines. If an API is still considered experimental or in development then
|
||
it should not be tested by Tempest until it is considered stable.
|