From 50fe5a6fcd1b3f146d3ce947c813303f63d7c577 Mon Sep 17 00:00:00 2001 From: Nigel Babu Date: Wed, 7 Feb 2018 11:04:12 +0530 Subject: Initial version of coding practices guide Change-Id: I382efec40d425c21ee5e0b91d0715fcdb07eae9f Signed-off-by: Nigel Babu --- docs/index.rst | 1 + docs/userguide/developer-guide.rst | 138 +++++++++++++++++++++++++++++++++++++ 2 files changed, 139 insertions(+) create mode 100644 docs/userguide/developer-guide.rst (limited to 'docs') diff --git a/docs/index.rst b/docs/index.rst index 909d70b97..0efe46933 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -15,6 +15,7 @@ User Guide :maxdepth: 3 userguide/intro + userguide/developer-guide API diff --git a/docs/userguide/developer-guide.rst b/docs/userguide/developer-guide.rst new file mode 100644 index 000000000..9ac300a05 --- /dev/null +++ b/docs/userguide/developer-guide.rst @@ -0,0 +1,138 @@ +####################################### +Developer Guide for Tests and Libraries +####################################### + +This is a collection of best practices for test and library developers in this +repo. Almost all the items are requirements for code to be merged. When +something is a recommendation, it will be marked appropriately. + +Follow the style guide +====================== + +Please follow the `PEP008 style guide`_. + +- Line Length: Limit all lines to a maximum of 79 characters. When you break + a line, use the tuple style:: + + calling_a_function("This is the first line" + "This is the second line") + +- Comments that contradict the code are worse than no comments. Always make + a priority of keeping the comments up-to-date when the code changes. All + comments in the code use the ``#`` format. + +- Conventions for writing good documentation strings ("docstrings") are + immortalized in `PEP 257`_. Only use the ``'''`` format for documentation + strings. They're written at the start of a function or class. + +- Please follow PEP8 guidelines for naming convenctions. + +Best Practices +============== + +- If you are not doing anything specific to your test in the ``setUp`` and + ``tearDown`` class, please do not declare them only to call the base class' + setUp and tearDown functions. + +- Use the `assert functions`_ provided in the unittest library. Beyond + ``assertTrue`` and ``assertEqual`` functions, there are also + ``assertIn`` and ``assertRegexpMatches`` functions. + +- Do **NOT** peer probe from your test or write code to check for peer probe + status. The infrastructure setup will do the peer probe for you. If you use + ``setup_volume`` from the ``GlusterBaseClass``, you are already checking + if the hosts are peer probed. There's ``validate_peers_are_connected`` in + ``GlusterBaseClass`` which can check peer probe status if you are not using + ``setup_volume``. Exception: Your test is a glusterd test which actually + tests peer probe functionality. + +- Please write `meaningful commit messages`_. + * The first line of the commit message should only have 72 characters and + should stand on its own. + * Leave a blank line and then have a detailed commit message. + * When you update a patch, please do not edit the original commit message + like would for Github. Changing it to "Fixed Review Comments" is a common + mistake. + +- Make sure the log messages are grammatically correct and have no spelling + mistakes. + +- Follow the Python `best practices`_ for writing tests. + +- Comment every step of the test case, log the test step, test result, failure + and success. + +- In the ``setUp``, ``tearDown``, ``setUpClass``, and + ``tearDownClass``, use Exceptions for unexpected results. In the test + itself, only use asserts for testing outcome of what you're testing. For + every other unexpected failure, raise an Exception. + +:: + + from glustolibs.gluster.gluster_base_class import GlusterBaseClass + + + class GlusterTestClass(GlusterBaseClass): + @classmethod + def setUpClass(cls): + GlusterBaseClass.setUpClass.im_func(self) + # Your code here + # Remove this function if you don't have set up steps to do + + def setUp(self): + GlusterBaseClass.setUp.im_func(self) + # Your code here + # Remove this function if you don't have set up steps to do + + def test_name_of_test_case1(self): + ''' + Docstring describing what this test will do + ''' + pass + + def test_name_of_test_case2(self): + ''' + Docstring describing what this test will do + ''' + pass + + def tearDown(self): + GlusterBaseClass.tearDown.im_func(self) + # Your code here + # Remove this function if you don't have set up steps to do + + @classmethod + def tearDownClass(cls): + GlusterBaseClass.tearDownClass.im_func(self) + # Your code here + # Remove this function if you don't have set up steps to do + +Submitting patches for review +============================= + +- Test the libraries and test case for all the volumes and mount protocols it + can run on before submitting the patch. + +- Check the test code against pep8 standards. We use flake8 for the lint on + Gerrit. If you want to make sure it passes, run it locally before submitting + a change. + +- Add the owners of the component to review the patch. A review from the + component owner is recommended but not required. + +- Only mark the patch as Verified +1 if the test is verified across on all + volume and mount protocols. + +- If the patch contains library modifications, then ensure they are well + tested and do not break current tests. + +- Patches are merged by the Gluster component maintainers and peers. + +.. note:: We recommend that the component owner reviews the any test which + touches their component, however the lack of a review from them does not + prevent a merge. + +.. _PEP008 style guide: https://www.python.org/dev/peps/pep-0008/ +.. _PEP257: https://www.python.org/dev/peps/pep-0257/ +.. _assert functions: https://docs.python.org/2/library/unittest.html#unittest.TestCase.assertEqual +.. _best practices: https://docs.python.org/2/library/unittest.html#test-cases -- cgit