From 79e85313aa611b599f19fea61c38ff3928e1fd59 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 18 Sep 2014 14:03:49 -0400 Subject: [PATCH] Write the outlines of a WritingTests.txt document Also, add some sample tests to be examples. --- doc/WritingTests.txt | 167 ++++++++++++++++++++++++++++++++++++++++ src/common/tortls.c | 4 +- src/common/tortls.h | 2 +- src/common/util.c | 2 +- src/common/util.h | 2 +- src/or/buffers.c | 2 +- src/test/test_buffers.c | 55 +++++++++++++ src/test/test_util.c | 30 ++++++++ 8 files changed, 258 insertions(+), 6 deletions(-) create mode 100644 doc/WritingTests.txt 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 filename. */ 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 };