Merge pull request #3503 from edx/db/process-updates

Reorganize information in CONTRIBUTING.rst, link to ReadTheDocs

docs/en_us/developers/source/index.rst

@@ -22,6 +22,7 @@ Contents:
     public_sandboxes.rst
     analytics.rst
     process/index
+    testing/index
 
 APIs
 -----

docs/en_us/developers/source/process/contributor.rst

@@ -18,7 +18,7 @@ and effort making a pull request.
 It’s also sometimes useful to submit a pull request even before the code is
 working properly, to make it easier to collect early feedback. To indicate to
 others that your pull request is not yet in a functional state, just prefix the
-pull request title with "WIP:" (which stands for Work In Progress).
+pull request title with "(WIP)" (which stands for Work In Progress).
 
 Once you’re ready to submit your changes in a pull request, check the following
 list of requirements to be sure that your pull request is ready to be reviewed:
@@ -95,5 +95,17 @@ if we do reject your pull request, we will explain why we aren’t taking it, an
 try to suggest other ways that you can accomplish the same result in a way that
 we will accept.
 
+Further Information
+-------------------
+For futher information on the pull request requirements, please see the following
+links:
+
+* :doc:`../testing`
+* :doc:`../testing/jenkins`
+* :doc:`../testing/code-coverage`
+* :doc:`../testing/code-quality`
+* `Python Guidelines <https://github.com/edx/edx-platform/wiki/Python-Guidelines>`_
+* `Javascript Guidelines <https://github.com/edx/edx-platform/wiki/Javascript-Guidelines>`_
+
 .. _contributor's agreement with edX: http://code.edx.org/individual-contributor-agreement.pdf
 .. _compatible licenses: https://github.com/edx/edx-platform/wiki/Licensing

docs/en_us/developers/source/testing/code-coverage.rst

+*************
+Code Coverage
+*************
+
+We measure which lines of our codebase are covered by unit tests using
+`coverage.py`_ for Python and `JSCover`_ for Javascript.
+
+Our codebase is far from perfect, but the goal is to steadily improve our coverage
+over time. To do this, we wrote a tool called `diff-cover`_ that will
+report which lines in your branch are not covered by tests, while ignoring
+other lines in the project that may not be covered. Using this tool,
+we can ensure that pull requests have a very high percentage of test coverage
+-- and ideally, they increase the test coverage of existing code, as well.
+
+To check the coverage of your pull request, just go to the top level of the
+edx-platform codebase and run::
+
+    $ rake coverage
+
+This will print a coverage report for your branch. We aim for
+a coverage report score of 95% or higher. We also encourage you to write
+acceptance tests as your changes require.
+
+.. _coverage.py: https://pypi.python.org/pypi/coverage
+.. _JSCover: http://tntim96.github.io/JSCover/
+.. _diff-cover: https://github.com/edx/diff-cover

docs/en_us/developers/source/testing/code-quality.rst

+************
+Code Quality
+************
+
+In order to keep our code as clear and readable as possible, we use various
+tools to assess the quality of pull requests:
+
+* We use the `pep8`_ tool to follow `PEP-8`_ guidelines
+* We use `pylint`_ for static analysis and uncovering trouble spots in our code
+
+Our codebase is far from perfect, but the goal is to steadily improve our quality
+over time. To do this, we wrote a tool called `diff-quality`_ that will
+only report on the quality violations on lines that have changed in a
+pull request. Using this tool, we can ensure that pull requests do not introduce
+any new quality violations -- and ideally, they clean up existing violations
+in the process of introducing other changes.
+
+To check the quality of your pull request, just go to the top level of the
+edx-platform codebase and run::
+
+    $ rake quality
+
+This will print a report of the quality violations that your branch has made.
+
+Although we try to be vigilant and resolve all quality violations, some Pylint
+violations are just too challenging to resolve, so we opt to ignore them via
+use of a pragma. A pragma tells Pylint to ignore the violation in the given
+line. An example is::
+
+    self.assertEquals(msg, form._errors['course_id'][0])  # pylint: disable=protected-access
+
+The pragma starts with a ``#`` two spaces after the end of the line. We prefer
+that you use the full name of the error (``pylint: disable=unused-argument`` as
+opposed to ``pylint: disable=W0613``), so it's more clear what you're disabling
+in the line.
+
+.. _PEP-8: http://legacy.python.org/dev/peps/pep-0008/
+.. _pep8: https://pypi.python.org/pypi/pep8
+.. _coverage.py: https://pypi.python.org/pypi/coverage
+.. _pylint: http://pylint.org/
+.. _diff-quality: https://github.com/edx/diff-cover

docs/en_us/developers/source/testing/index.rst

+*******
+Testing
+*******
+
+Testing is something that we take very seriously at edX: we even have a
+"test engineering" team at edX devoted purely to making our testing
+infrastructure even more awesome.
+
+This file is currently a stub: to find out more about our testing infrastructure,
+check out the `testing.md`_ file on Github.
+
+.. toctree::
+    :maxdepth: 2
+
+    jenkins
+    code-coverage
+    code-quality
+
+.. _testing.md: https://github.com/edx/edx-platform/blob/master/docs/en_us/internal/testing.md

docs/en_us/developers/source/testing/jenkins.rst

+*******
+Jenkins
+*******
+
+`Jenkins`_ is an open source continuous integration server. edX has a Jenkins
+installation specifically for testing pull requests to our open source software
+project, including edx-platform. Before a pull request can be merged, Jenkins
+must run all the tests for that pull request: this is known as a "build".
+If even one test in the build fails, then the entire build is considered a
+failure. Pull requests cannot be merged until they have a passing build.
+
+Kicking Off Builds
+==================
+
+Jenkins has the ability to automatically detect new pull requests and changed
+pull requests on Github, and it can automatically run builds in response to
+these events. We have Jenkins configured to automatically run builds for all
+pull requests from core committers; however, Jenkins will *not* automatically
+run builds for new contributors, so a community manager will need to manually
+kick off a build for a pull request from a new contributor.
+
+The reason for this distinction is a matter of trust. Running a build means that
+Jenkins will execute all the code in the pull request. A pull request can
+contain any code whatsoever: if we allowed Jenkins to automatically build every
+pull request, then a malicious developer could make our Jenkins server do whatever
+he or she wanted. Before kicking off a build, community managers look at the
+code changes to verify that they are not malicious; this protects us from nasty
+people.
+
+Once a contributor has submitted a few pull requests, they can request to be
+added to the Jenkins whitelist: this is a special list of people that Jenkins
+*will* kick off builds for automatically. If the community managers feel that
+the contributor is trustworthy, then they will grant the request, which will
+make future development faster and easier for both the contributor and edX. If
+a contibutor shows that they can not be trusted for some reason, they will be
+removed from this whitelist.
+
+Failed Builds
+=============
+
+Click on the build to be brought to the build page. You'll see a matrix of blue
+and red dots; the red dots indicate what section failing tests were present in.
+You can click on the test name to be brought to an error trace that explains
+why the tests fail. Please address the failing tests before requesting a new
+build on your branch. If the failures appear to not have anything to do with
+your code, it may be the case that the master branch is failing. You can ask
+your reviewers for advice in this scenario.
+
+If the build says "Unstable" but passes all tests, you have introduced too many
+pep8 and pylint violations. Please refer to the documentation for :doc:`code-quality`
+and clean up the code.
+
+Successful Builds
+=================
+
+If all the tests pass, the "Diff Coverage" and "Diff Quality" reports are
+generated. Click on the "View Reports" link on your pull request to be brought
+to the Jenkins report page. In a column on the left side of the page are a few
+links, including "Diff Coverage Report" and "Diff Quality Report". View each of
+these reports (making note that the Diff Quality report has two tabs - one for
+pep8, and one for Pylint).
+
+Make sure your quality coverage is 100% and your test coverage is at least 95%.
+Adjust your code appropriately if these metrics are not high enough. Be sure to
+ask your reviewers for advice if you need it.
+
+
+.. _Jenkins: http://jenkins-ci.org/