Write the outlines of a WritingTests.txt document

Also, add some sample tests to be examples.

8 files changed, 258 insertions, 6 deletions

diff --git a/doc/WritingTests.txt b/doc/WritingTests.txt
new file mode 100644
index 0000000000..ab6d084ff9
--- /dev/null
+++ b/doc/WritingTests.txt
@@ -0,0 +1,167 @@
+
+Writing tests for Tor: an incomplete guide
+==========================================
+
+Tor uses a variety of testing frameworks and methodologies to try to
+keep from introducing bugs.  The major ones are:
+
+   1. Unit tests written in C and shipped with the Tor distribution.
+
+   2. Integration tests written in Python and shipped with the Tor
+      distribution.
+
+   3. Integration tests written in Python and shipped with the Stem
+      library.  Some of these use the Tor controller protocol.
+
+   4. System tests written in Python and SH, and shipped with the
+      Chutney package.  These work by running many instances of Tor
+      locally, and sending traffic through them.
+
+   5. The Shadow network simulator.
+
+How to run these tests
+----------------------
+
+=== The easy version
+
+To run all the tests that come bundled with Tor, run "make check"
+
+To run the Stem tests as well, fetch stem from the git repository,
+set STEM_SOURCE_DIR to the checkout, and run "make test-stem".
+
+To run the Chutney tests as well, fetch chutney from the git repository,
+set CHUTNEY_PATH to the checkout, and run "make test-network".
+
+=== Running particular subtests
+
+XXXX WRITEME
+
+=== Finding test coverage
+
+When you configure Tor with the --enable-coverage option, it should
+build with support for coverage in the unit tests, and in a special
+"tor-cov" binary.  If you launch
+
+XXXX "make test-network" doesn't know about "tor-cov"; you don't get
+XXXX coverage from that yet, unless you do "cp src/or/tor-cov
+XXXX src/or/tor" before you run it.
+
+What kinds of test should I write?
+----------------------------------
+
+XXXX writeme.
+
+
+Unit and regression tests: Does this function do what it's supposed to?
+-----------------------------------------------------------------------
+
+Most of Tor's unit tests are made using the "tinytest" testing framework.
+You can see a guide to using it in the tinytest manual at
+
+   https://github.com/nmathewson/tinytest/blob/master/tinytest-manual.md
+
+To add a new test of this kind, either edit an existing C file in src/test/,
+or create a new C file there.  Each test is a single function that must
+be indexed in the table at the end of the file.  We use the label "done:" as
+a cleanup point for all test functions.
+
+(Make sure you read tinytest-manual.md before proceeding.)
+
+I use the term "unit test" and "regression tests" very sloppily here.
+
+=== A simple example
+
+Here's an example of a test function for a simple function in util.c:
+
+    static void
+    test_util_writepid(void *arg)
+    {
+      (void) arg;
+
+      char *contents = NULL;
+      const char *fname = get_fname("tmp_pid");
+      unsigned long pid;
+      char c;
+
+      write_pidfile(fname);
+
+      contents = read_file_to_str(fname, 0, NULL);
+      tt_assert(contents);
+
+      int n = sscanf(contents, "%lu\n%c", &pid, &c);
+      tt_int_op(n, OP_EQ, 1);
+      tt_int_op(pid, OP_EQ, getpid());
+
+    done:
+      tor_free(contents);
+    }
+
+This should look pretty familier to you if you've read the tinytest
+manual.  One thing to note here is that we use the testing-specific
+function "get_fname" to generate a file with respect to a temporary
+directory that the tests use.  You don't need to delete the file;
+it will get removed when the tests are done.
+
+Also note our use of OP_EQ instead of == in the tt_int_op() calls.
+We define OP_* macros to use instead of the binary comparison
+operators so that analysis tools can more easily parse our code.
+(Coccinelle really hates to see == used as a macro argument.)
+
+Finally, remember that by convention, all *_free() functions that
+Tor defines are defined to accept NULL harmlessly.  Thus, you don't
+need to say "if (contents)" in the cleanup block.
+
+=== Exposing static functions for testing
+
+Sometimes you need to test a function, but you don't want to expose
+it outside its usual module.
+
+To support this, Tor's build system compiles a testing version of
+teach module, with extra identifiers exposed.  If you want to
+declare a function as static but available for testing, use the
+macro "STATIC" instead of "static."  Then, make sure there's a
+macro-protected declaration of the function in the module's header.
+
+For example, crypto_curve25519.h contains:
+
+#ifdef CRYPTO_CURVE25519_PRIVATE
+STATIC int curve25519_impl(uint8_t *output, const uint8_t *secret,
+                           const uint8_t *basepoint);
+#endif
+
+The crypto_curve25519.c file and the test_crypto.c file both define
+CRYPTO_CURVE25519_PRIVATE, so they can see this declaration.
+
+=== Mock functions for testing in isolation
+
+Often we want to test that a function works right, but the function depends
+on other functions whose behavior is hard to observe, or whose 
+
+XXXX WRITEME
+
+=== Advanced techniques: Namespaces
+
+
+XXXX write this.  danah boyd made us some really awesome stuff here.
+
+
+Integration tests: Calling Tor from the outside
+-----------------------------------------------
+
+XXXX WRITEME
+
+Writing integration tests with Stem
+-----------------------------------
+
+XXXX WRITEME
+
+System testing with Chutney
+---------------------------
+
+XXXX WRITEME
+
+Who knows what evil lurks in the timings of networks? The Shadow knows!
+-----------------------------------------------------------------------
+
+XXXX WRITEME
+
diff --git a/src/common/tortls.c b/src/common/tortls.c
index ca629135a6..43fa7f884d 100644
--- a/src/common/tortls.c
+++ b/src/common/tortls.c
@@ -2068,8 +2068,8 @@ tor_tls_free(tor_tls_t *tls)
  * number of characters read.  On failure, returns TOR_TLS_ERROR,
  * TOR_TLS_CLOSE, TOR_TLS_WANTREAD, or TOR_TLS_WANTWRITE.
  */
-int
-tor_tls_read(tor_tls_t *tls, char *cp, size_t len)
+MOCK_IMPL(int,
+tor_tls_read,(tor_tls_t *tls, char *cp, size_t len))
 {
   int r, err;
   tor_assert(tls);
diff --git a/src/common/tortls.h b/src/common/tortls.h
index f8c6d5913b..2aac8af18e 100644
--- a/src/common/tortls.h
+++ b/src/common/tortls.h
@@ -77,7 +77,7 @@ int tor_tls_verify(int severity, tor_tls_t *tls, crypto_pk_t **identity);
 int tor_tls_check_lifetime(int severity,
                            tor_tls_t *tls, int past_tolerance,
                            int future_tolerance);
-int tor_tls_read(tor_tls_t *tls, char *cp, size_t len);
+MOCK_DECL(int, tor_tls_read, (tor_tls_t *tls, char *cp, size_t len));
 int tor_tls_write(tor_tls_t *tls, const char *cp, size_t n);
 int tor_tls_handshake(tor_tls_t *tls);
 int tor_tls_finish_handshake(tor_tls_t *tls);
diff --git a/src/common/util.c b/src/common/util.c
index f7baab0791..037acad87b 100644
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -3538,7 +3538,7 @@ finish_daemon(const char *cp)
 /** Write the current process ID, followed by NL, into <b>filename</b>.
  */
 void
-write_pidfile(char *filename)
+write_pidfile(const char *filename)
 {
   FILE *pidfile;
 
diff --git a/src/common/util.h b/src/common/util.h
index 1b8fc74db5..bac4097917 100644
--- a/src/common/util.h
+++ b/src/common/util.h
@@ -410,7 +410,7 @@ int path_is_relative(const char *filename);
 /* Process helpers */
 void start_daemon(void);
 void finish_daemon(const char *desired_cwd);
-void write_pidfile(char *filename);
+void write_pidfile(const char *filename);
 
 /* Port forwarding */
 void tor_check_port_forwarding(const char *filename,
diff --git a/src/or/buffers.c b/src/or/buffers.c
index ca0e815e33..6d1333844d 100644
--- a/src/or/buffers.c
+++ b/src/or/buffers.c
@@ -842,7 +842,7 @@ read_to_buf_tls(tor_tls_t *tls, size_t at_most, buf_t *buf)
     if (r < 0)
       return r; /* Error */
     tor_assert(total_read+r < INT_MAX);
-     total_read += r;
+    total_read += r;
     if ((size_t)r < readlen) /* eof, block, or no more to read. */
       break;
   }
diff --git a/src/test/test_buffers.c b/src/test/test_buffers.c
index 101f448472..8ca8be567f 100644
--- a/src/test/test_buffers.c
+++ b/src/test/test_buffers.c
@@ -714,6 +714,59 @@ test_buffers_zlib_fin_at_chunk_end(void *arg)
   tor_free(msg);
 }
 
+const uint8_t *tls_read_ptr;
+int n_remaining;
+int next_reply_val[16];
+
+static int
+mock_tls_read(tor_tls_t *tls, char *cp, size_t len)
+{
+  (void)tls;
+  int rv = next_reply_val[0];
+  if (rv > 0) {
+    int max = rv > (int)len ? (int)len : rv;
+    if (max > n_remaining)
+      max = n_remaining;
+    memcpy(cp, tls_read_ptr, max);
+    rv = max;
+    n_remaining -= max;
+    tls_read_ptr += max;
+  }
+
+  memmove(next_reply_val, next_reply_val + 1, 15*sizeof(int));
+  return rv;
+}
+
+static void
+test_buffers_tls_read_mocked(void *arg)
+{
+  uint8_t *mem;
+  buf_t *buf;
+  (void)arg;
+
+  mem = tor_malloc(64*1024);
+  crypto_rand((char*)mem, 64*1024);
+  tls_read_ptr = mem;
+  n_remaining = 64*1024;
+
+  MOCK(tor_tls_read, mock_tls_read);
+
+  buf = buf_new();
+
+  next_reply_val[0] = 1024;
+  tt_int_op(128, ==, read_to_buf_tls(NULL, 128, buf));
+
+  next_reply_val[0] = 5000;
+  next_reply_val[1] = 5000;
+  tt_int_op(6000, ==, read_to_buf_tls(NULL, 6000, buf));
+
+
+ done:
+  UNMOCK(tor_tls_read);
+  tor_free(mem);
+  buf_free(buf);
+}
+
 struct testcase_t buffer_tests[] = {
   { "basic", test_buffers_basic, TT_FORK, NULL, NULL },
   { "copy", test_buffer_copy, TT_FORK, NULL, NULL },
@@ -726,6 +779,8 @@ struct testcase_t buffer_tests[] = {
   { "zlib_fin_with_nil", test_buffers_zlib_fin_with_nil, TT_FORK, NULL, NULL },
   { "zlib_fin_at_chunk_end", test_buffers_zlib_fin_at_chunk_end, TT_FORK,
     NULL, NULL},
+  { "tls_read_mocked", test_buffers_tls_read_mocked, 0,
+    NULL, NULL },
   END_OF_TESTCASES
 };
 
diff --git a/src/test/test_util.c b/src/test/test_util.c
index 15470e8efa..3bebe60490 100644
--- a/src/test/test_util.c
+++ b/src/test/test_util.c
@@ -4905,6 +4905,35 @@ test_util_ipv4_validation(void *arg)
   return;
 }
 
+static void
+test_util_writepid(void *arg)
+{
+  (void) arg;
+
+  char *contents = NULL;
+  const char *fname = get_fname("tmp_pid");
+  unsigned long pid;
+  char c;
+
+  write_pidfile(fname);
+
+  contents = read_file_to_str(fname, 0, NULL);
+  tt_assert(contents);
+
+  int n = sscanf(contents, "%lu\n%c", &pid, &c);
+  tt_int_op(n, OP_EQ, 1);
+  tt_uint_op(pid, OP_EQ,
+#ifdef _WIN32
+             _getpid()
+#else
+             getpid()
+#endif
+             );
+
+ done:
+  tor_free(contents);
+}
+
 struct testcase_t util_tests[] = {
   UTIL_LEGACY(time),
   UTIL_TEST(parse_http_time, 0),
@@ -4981,6 +5010,7 @@ struct testcase_t util_tests[] = {
   UTIL_TEST(max_mem, 0),
   UTIL_TEST(hostname_validation, 0),
   UTIL_TEST(ipv4_validation, 0),
+  UTIL_TEST(writepid, 0),
   END_OF_TESTCASES
 };