From f89ab81c38a7ffc87303b10e156759e25c09abdb Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Tue, 10 Jul 2018 20:34:02 -0400 Subject: [PATCH] docs: Add information on test removal/interop to REVIEWING This patch set adds information on test removal, relocation, renaming to REVIEWING because it is important that such actions do not break interop. Interop is closely tied to Tempest because it directly references Tempest tests that are not only expected to exist, but to also work. The same is true of breaking blacklist/whitelist references to Tempest tests, this is also included in the new documentation section. It is important that there be REVIEWING guidelines in place to assist reviewers understand this importance. Also references are included for defcore/interop to help users find more information on these topics. Currently interop is only mentioned in 1 place in Tempest [0] and yet there is little information about it. This patchset aims to make it easier to find more information about it for reviewers and users alike. [0] http://git.openstack.org/cgit/openstack/tempest/tree/doc/source/test_removal.rst Change-Id: Ifbde674b42355077fcd8daa07be8be1248b77b0f --- REVIEWING.rst | 37 ++++++++++++++++++++++++++++++++++++- doc/source/test_removal.rst | 9 +++++++-- 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/REVIEWING.rst b/REVIEWING.rst index a8801814d1..8a1e1526b4 100644 --- a/REVIEWING.rst +++ b/REVIEWING.rst @@ -99,6 +99,39 @@ level docstring linking to the API ref doc in the API tests and a docstring for scenario tests this is up to the reviewers discretion whether a docstring is required or not. + +Test Removal and Refactoring +---------------------------- +Make sure that any test that is renamed, relocated (e.g. moved to another +class), or removed does not belong to the `interop`_ testing suite -- which +includes a select suite of Tempest tests for the purposes of validating that +OpenStack vendor clouds are interoperable -- or a project's `whitelist`_ or +`blacklist`_ files. + +It is of critical importance that no interop, whitelist or blacklist test +reference be broken by a patch set introduced to Tempest that renames, +relocates or removes a referenced test. + +Please check the existence of code which references Tempest tests with: +http://codesearch.openstack.org/ + +Interop +^^^^^^^ +Make sure that modifications to an `interop`_ test are backwards-compatible. +This means that code modifications to tests should not undermine the quality of +the validation currently performed by the test or significantly alter the +behavior of the test. + +Removal +^^^^^^^ +Reference the :ref:`test-removal` guidelines for understanding best practices +associated with test removal. + +.. _interop: https://www.openstack.org/brand/interop +.. _whitelist: https://docs.openstack.org/tempest/latest/run.html#test-selection +.. _blacklist: https://docs.openstack.org/tempest/latest/run.html#test-selection + + Release Notes ------------- Release notes are how we indicate to users and other consumers of Tempest what @@ -113,16 +146,18 @@ something extra. .. _reno: https://docs.openstack.org/reno/latest/ + Deprecated Code --------------- Sometimes we have some bugs in deprecated code. Basically, we leave it. Because we don't need to maintain it. However, if the bug is critical, we might need to fix it. When it will happen, we will deal with it on a case-by-case basis. + When to approve --------------- * Every patch needs two +2s before being approved. -* Its ok to hold off on an approval until a subject matter expert reviews it +* It's ok to hold off on an approval until a subject matter expert reviews it * If a patch has already been approved but requires a trivial rebase to merge, you do not have to wait for a second +2, since the patch has already had two +2s. diff --git a/doc/source/test_removal.rst b/doc/source/test_removal.rst index b22503b7ad..917b014c22 100644 --- a/doc/source/test_removal.rst +++ b/doc/source/test_removal.rst @@ -1,3 +1,5 @@ +.. _test-removal: + Tempest Test Removal Procedure ============================== @@ -107,16 +109,19 @@ intent of this prong was mostly for refstack/defcore and also for things that running on the stable branches. We don't want to remove any tests if that would break our API consistency checking between releases, or something that defcore/refstack is depending on being in Tempest. It's worth pointing out -that if a test is used in defcore as part of interop testing then it will +that if a test is used in `defcore`_ as part of `interop`_ testing then it will probably have continuing value being in Tempest as part of the integration/integrated tests in general. This is one area where some overlap is expected between testing in projects and Tempest, which is not a bad thing. +.. _defcore: https://wiki.openstack.org/wiki/Governance/InteropWG +.. _interop: https://www.openstack.org/brand/interop + Discussing the 3rd prong """""""""""""""""""""""" There are 2 approaches to addressing the 3rd prong. Either it can be raised -during a qa meeting during the Tempest discussion. Please put it on the agenda +during a QA meeting during the Tempest discussion. Please put it on the agenda well ahead of the scheduled meeting. Since the meeting time will be well known ahead of time anyone who depends on the tests will have ample time beforehand to outline any concerns on the before the meeting. To give ample time for