From b2ada7e706206ca2f7d0e79b72d6debea74998b5 Mon Sep 17 00:00:00 2001 From: Tim Abbott Date: Tue, 4 Jul 2017 13:13:38 -0700 Subject: [PATCH] docs: Expand documentation on Internet access in tests. --- docs/testing-with-django.md | 8 ++++- docs/testing.md | 60 +++++++++++++++++++++++++++++-------- tools/test-backend | 5 ++-- 3 files changed, 56 insertions(+), 17 deletions(-) diff --git a/docs/testing-with-django.md b/docs/testing-with-django.md index ee73a7d25e..8bf38dd4dc 100644 --- a/docs/testing-with-django.md +++ b/docs/testing-with-django.md @@ -168,7 +168,9 @@ We use mocks and stubs for all the typical reasons: - to more precisely test the target code - to stub out calls to third-party services -- to make it so that you can run your tests on the airplane without wifi +- to make it so that you can [run the Zulip tests on the airplane without wifi][no-internet] + +[no-internet]: testing.html#internet-access-inside-test-suites For mocking we generally use the "mock" library and use `mock.patch` as a context manager or decorator. We also take advantage of some context managers @@ -184,6 +186,10 @@ from Django as well as our own custom helpers. Here is an example: Follow [this link](settings.html#testing-non-default-settings) for more information on the "settings" context manager. +A common use is to prevent a call to a third-party service from using +the Internet; `git grep mock.patch | grep requests` is a good way to +find several examples of doing this. + ### Template tests In [zerver/tests/test_templates.py](https://github.com/zulip/zulip/blob/master/zerver/tests/test_templates.py) diff --git a/docs/testing.md b/docs/testing.md index 21ddc63250..ec6bed8ccc 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -79,26 +79,60 @@ if you're working on new database migrations. To do this, run: [lxc-sf]: https://github.com/fgrehm/vagrant-lxc/wiki/FAQ#help-my-shared-folders-have-the-wrong-owner -### Internet access inside test suits +### Internet access inside test suites -Zulip has a policy of requiring its backend tests to not depend upon Internet -connectivity. We currently enforce this policy by overriding library functions -which might be used to access internet (eg. `httplib2.Http().request`). Though -this might not be a full proof blockade to internet access but we intend to -improve upon this in near time. +As a policy matter, the Zulip test suites should never make outgoing +HTTP or other network requests. This is important for 2 major +reasons: -- If you are seeing test failures with tracebacks as below, you should -probably mock the element trying to access the network. You can consult our -[guide](/docs/testing-with-django.htmls#mocks-and-stubs) on -mocking if you want to learn how to write mockups. +* Tests that make outgoing Internet requests will fail when the user + isn't on the Internet. +* Tests that make outgoing Internet requests often have a hidden + dependency on the uptime of a third-party service, and will fail + nondeterministically if that service has a temporary outage. + Nondeterministically failing tests can be a big waste of + developer time, and we try to avoid them wherever possible. + +As a result, Zulip's major test suites should never access the +Internet directly. Since code in Zulip does need to access the +Internet (e.g. to access various third-party APIs), this means that +the Zulip tests use mocking to basically hardcode (for the purposes of +the test) what responses should be used for any outgoing Internet +requests that Zulip would make in the code path being tested. + +This is easy to do using test fixtures (a fancy word for fixed data +used in tests) and the `mock.patch` function to specify what HTTP +response should be used by the tests for every outgoing HTTP (or other +network) request. Consult +[our guide on mocking](testing-with-django.html#mocks-and-stubs) to +learn how to mock network requests easily; there are also a number of +examples throughout the codebase. + +We partially enforce this policy in the main Django/backend test suite +by overriding certain library functions that are used in outgoing HTTP +code paths (`httplib2.Http().request`, `requests.request`, etc.) to +throw an exception in the backend tests. While this is enforcement is +not complete (there a lot of other ways to use the Internet from +Python), it is easy to do and catches most common cases of new code +dependning on Internet access. + +This enforcement code results in the following exception: ``` File "tools/test-backend", line 120, in internet_guard - raise Exception("Zulip doesn't allow network access to test suits." - Exception: Zulip doesn't allow network access to test suits. - You need to mock any network access calls in testsuits. + raise Exception("Outgoing network requests are not allowed in the Zulip tests." + Exception: Outgoing network requests are not allowed in the Zulip tests. + ... ``` +#### Documentation tests + +The one exception to this policy is our documentation tests, which +will attempt to verify that the links included in our documentation +aren't broken. Those tests end up failing nondeterministically fairly +often, which is unfortunate, but there's simply no other correct way +to verify links other than attempting to access them. + ## Schema and initial data changes If you change the database schema or change the initial test data, you diff --git a/tools/test-backend b/tools/test-backend index d9902151c7..43a8308c4a 100755 --- a/tools/test-backend +++ b/tools/test-backend @@ -118,9 +118,8 @@ def block_internet(): # httplib2 to access internet. def internet_guard(*args, **kwargs): # type: (*Any, **Any) -> None - raise Exception("Zulip doesn't allow network access to test suits. " - "You need to mock any network access calls in test" - "suits. Checkout our docs on testing here for details." + raise Exception("Outgoing network requests are not allowed in the Zulip tests. " + "More details and advice are available here:" "https://zulip.readthedocs.io/en/latest/testing.html#internet-access-inside-test-suits") httplib2.Http.request = internet_guard