Skip to content

Commit

Permalink
Merge branch 'disable_ns_macro'
Browse files Browse the repository at this point in the history
  • Loading branch information
@nmathewson
nmathewson committed Jan 9, 2020
2 parents 93894fb + 079912d commit 5888db4
Show file tree
Hide file tree
Showing 17 changed files with 1,026 additions and 1,130 deletions.
4 changes: 4 additions & 0 deletions changes/ticket32887
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
o Code simplification and refactoring:
- Remove underused NS*() macros from test code: they make our
tests more confusing, especially for code-formatting tools.
Closes ticket 32887.
97 changes: 0 additions & 97 deletions src/test/test.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,103 +81,6 @@ struct crypto_pk_t *pk_generate(int idx);
void init_pregenerated_keys(void);
void free_pregenerated_keys(void);

#define US2_CONCAT_2__(a, b) a ## __ ## b
#define US_CONCAT_2__(a, b) a ## _ ## b
#define US_CONCAT_3__(a, b, c) a ## _ ## b ## _ ## c
#define US_CONCAT_2_(a, b) US_CONCAT_2__(a, b)
#define US_CONCAT_3_(a, b, c) US_CONCAT_3__(a, b, c)

/*
* These macros are helpful for streamlining the authorship of several test
* cases that use mocks.
*
* The pattern is as follows.
* * Declare a top level namespace:
* #define NS_MODULE foo
*
* * For each test case you want to write, create a new submodule in the
* namespace. All mocks and other information should belong to a single
* submodule to avoid interference with other test cases.
* You can simply name the submodule after the function in the module you
* are testing:
* #define NS_SUBMODULE some_function
* or, if you're wanting to write several tests against the same function,
* ie., you are testing an aspect of that function, you can use:
* #define NS_SUBMODULE ASPECT(some_function, behavior)
*
* * Declare all the mocks you will use. The NS_DECL macro serves to declare
* the mock in the current namespace (defined by NS_MODULE and NS_SUBMODULE).
* It behaves like MOCK_DECL:
* NS_DECL(int, dependent_function, (void *));
* Here, dependent_function must be declared and implemented with the
* MOCK_DECL and MOCK_IMPL macros. The NS_DECL macro also defines an integer
* global for use for tracking how many times a mock was called, and can be
* accessed by CALLED(mock_name). For example, you might put
* CALLED(dependent_function)++;
* in your mock body.
*
* * Define a function called NS(main) that will contain the body of the
* test case. The NS macro can be used to reference a name in the current
* namespace.
*
* * In NS(main), indicate that a mock function in the current namespace,
* declared with NS_DECL is to override that in the global namespace,
* with the NS_MOCK macro:
* NS_MOCK(dependent_function)
* Unmock with:
* NS_UNMOCK(dependent_function)
*
* * Define the mocks with the NS macro, eg.,
* int
* NS(dependent_function)(void *)
* {
* CALLED(dependent_function)++;
* }
*
* * In the struct testcase_t array, you can use the TEST_CASE and
* TEST_CASE_ASPECT macros to define the cases without having to do so
* explicitly nor without having to reset NS_SUBMODULE, eg.,
* struct testcase_t foo_tests[] = {
* TEST_CASE_ASPECT(some_function, behavior),
* ...
* END_OF_TESTCASES
* which will define a test case named "some_function__behavior".
*/

#define NAME_TEST_(name) #name
#define NAME_TEST(name) NAME_TEST_(name)
#define ASPECT(test_module, test_name) US2_CONCAT_2__(test_module, test_name)
#ifndef COCCI
#define TEST_CASE(function) \
{ \
NAME_TEST(function), \
NS_FULL(NS_MODULE, function, test_main), \
TT_FORK, \
NULL, \
NULL, \
}
#define TEST_CASE_ASPECT(function, aspect) \
{ \
NAME_TEST(ASPECT(function, aspect)), \
NS_FULL(NS_MODULE, ASPECT(function, aspect), test_main), \
TT_FORK, \
NULL, \
NULL, \
}
#endif /* !defined(COCCI) */

#define NS(name) US_CONCAT_3_(NS_MODULE, NS_SUBMODULE, name)
#define NS_FULL(module, submodule, name) US_CONCAT_3_(module, submodule, name)

#define CALLED(mock_name) US_CONCAT_2_(NS(mock_name), called)
#ifndef COCCI
#define NS_DECL(retval, mock_fn, args) \
extern int CALLED(mock_fn); \
static retval NS(mock_fn) args; int CALLED(mock_fn) = 0
#define NS_MOCK(name) MOCK(name, NS(name))
#endif /* !defined(COCCI) */
#define NS_UNMOCK(name) UNMOCK(name)

extern const struct testcase_setup_t passthrough_setup;
extern const struct testcase_setup_t ed25519_test_setup;

Expand Down
16 changes: 6 additions & 10 deletions src/test/test_accounting.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,16 @@

#include "app/config/or_state_st.h"

#define NS_MODULE accounting

#define NS_SUBMODULE limits

/*
* Test to make sure accounting triggers hibernation
* correctly with both sum or max rules set
*/

static or_state_t *or_state;
NS_DECL(or_state_t *, get_or_state, (void));
static or_state_t * acct_limits_get_or_state(void);
ATTR_UNUSED static int acct_limits_get_or_state_called = 0;
static or_state_t *
NS(get_or_state)(void)
acct_limits_get_or_state(void)
{
return or_state;
}
Expand All @@ -35,7 +32,8 @@ test_accounting_limits(void *arg)
time_t fake_time = time(NULL);
(void) arg;

NS_MOCK(get_or_state);
MOCK(get_or_state,
acct_limits_get_or_state);
or_state = or_state_new();

options->AccountingMax = 100;
Expand Down Expand Up @@ -94,12 +92,10 @@ test_accounting_limits(void *arg)

goto done;
done:
NS_UNMOCK(get_or_state);
UNMOCK(get_or_state);
or_state_free(or_state);
}

#undef NS_SUBMODULE

struct testcase_t accounting_tests[] = {
{ "bwlimits", test_accounting_limits, TT_FORK, NULL, NULL },
END_OF_TESTCASES
Expand Down
2 changes: 0 additions & 2 deletions src/test/test_compat_libevent.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@

#include "test/log_test_helpers.h"

#define NS_MODULE compat_libevent

static void
test_compat_libevent_logging_callback(void *ignored)
{
Expand Down
64 changes: 33 additions & 31 deletions src/test/test_dir.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,6 @@
#include <unistd.h>
#endif

#define NS_MODULE dir

static networkstatus_t *
networkstatus_parse_vote_from_string_(const char *s,
const char **eos_out,
Expand Down Expand Up @@ -5474,15 +5472,15 @@ test_dir_conn_purpose_to_string(void *data)
teardown_capture_of_logs();
}

NS_DECL(int,
public_server_mode, (const or_options_t *options));
static int dir_tests_public_server_mode(const or_options_t *options);
ATTR_UNUSED static int dir_tests_public_server_mode_called = 0;

static int
NS(public_server_mode)(const or_options_t *options)
dir_tests_public_server_mode(const or_options_t *options)
{
(void)options;

if (CALLED(public_server_mode)++ == 0) {
if (dir_tests_public_server_mode_called++ == 0) {
return 1;
}

Expand All @@ -5496,55 +5494,56 @@ test_dir_should_use_directory_guards(void *data)
char *errmsg = NULL;
(void)data;

NS_MOCK(public_server_mode);
MOCK(public_server_mode,
dir_tests_public_server_mode);

options = options_new();
options_init(options);

tt_int_op(should_use_directory_guards(options), OP_EQ, 0);
tt_int_op(CALLED(public_server_mode), OP_EQ, 1);
tt_int_op(dir_tests_public_server_mode_called, OP_EQ, 1);

options->UseEntryGuards = 1;
options->DownloadExtraInfo = 0;
options->FetchDirInfoEarly = 0;
options->FetchDirInfoExtraEarly = 0;
options->FetchUselessDescriptors = 0;
tt_int_op(should_use_directory_guards(options), OP_EQ, 1);
tt_int_op(CALLED(public_server_mode), OP_EQ, 2);
tt_int_op(dir_tests_public_server_mode_called, OP_EQ, 2);

options->UseEntryGuards = 0;
tt_int_op(should_use_directory_guards(options), OP_EQ, 0);
tt_int_op(CALLED(public_server_mode), OP_EQ, 3);
tt_int_op(dir_tests_public_server_mode_called, OP_EQ, 3);
options->UseEntryGuards = 1;

options->DownloadExtraInfo = 1;
tt_int_op(should_use_directory_guards(options), OP_EQ, 0);
tt_int_op(CALLED(public_server_mode), OP_EQ, 4);
tt_int_op(dir_tests_public_server_mode_called, OP_EQ, 4);
options->DownloadExtraInfo = 0;

options->FetchDirInfoEarly = 1;
tt_int_op(should_use_directory_guards(options), OP_EQ, 0);
tt_int_op(CALLED(public_server_mode), OP_EQ, 5);
tt_int_op(dir_tests_public_server_mode_called, OP_EQ, 5);
options->FetchDirInfoEarly = 0;

options->FetchDirInfoExtraEarly = 1;
tt_int_op(should_use_directory_guards(options), OP_EQ, 0);
tt_int_op(CALLED(public_server_mode), OP_EQ, 6);
tt_int_op(dir_tests_public_server_mode_called, OP_EQ, 6);
options->FetchDirInfoExtraEarly = 0;

options->FetchUselessDescriptors = 1;
tt_int_op(should_use_directory_guards(options), OP_EQ, 0);
tt_int_op(CALLED(public_server_mode), OP_EQ, 7);
tt_int_op(dir_tests_public_server_mode_called, OP_EQ, 7);
options->FetchUselessDescriptors = 0;

done:
NS_UNMOCK(public_server_mode);
UNMOCK(public_server_mode);
or_options_free(options);
tor_free(errmsg);
}

NS_DECL(void,
directory_initiate_request, (directory_request_t *req));
static void dir_tests_directory_initiate_request(directory_request_t *req);
ATTR_UNUSED static int dir_tests_directory_initiate_request_called = 0;

static void
test_dir_should_not_init_request_to_ourselves(void *data)
Expand All @@ -5554,7 +5553,8 @@ test_dir_should_not_init_request_to_ourselves(void *data)
crypto_pk_t *key = pk_generate(2);
(void) data;

NS_MOCK(directory_initiate_request);
MOCK(directory_initiate_request,
dir_tests_directory_initiate_request);

clear_dir_servers();
routerlist_free_all();
Expand All @@ -5569,15 +5569,15 @@ test_dir_should_not_init_request_to_ourselves(void *data)
dir_server_add(ourself);

directory_get_from_all_authorities(DIR_PURPOSE_FETCH_STATUS_VOTE, 0, NULL);
tt_int_op(CALLED(directory_initiate_request), OP_EQ, 0);
tt_int_op(dir_tests_directory_initiate_request_called, OP_EQ, 0);

directory_get_from_all_authorities(DIR_PURPOSE_FETCH_DETACHED_SIGNATURES, 0,
NULL);

tt_int_op(CALLED(directory_initiate_request), OP_EQ, 0);
tt_int_op(dir_tests_directory_initiate_request_called, OP_EQ, 0);

done:
NS_UNMOCK(directory_initiate_request);
UNMOCK(directory_initiate_request);
clear_dir_servers();
routerlist_free_all();
crypto_pk_free(key);
Expand All @@ -5591,7 +5591,8 @@ test_dir_should_not_init_request_to_dir_auths_without_v3_info(void *data)
| MICRODESC_DIRINFO;
(void) data;

NS_MOCK(directory_initiate_request);
MOCK(directory_initiate_request,
dir_tests_directory_initiate_request);

clear_dir_servers();
routerlist_free_all();
Expand All @@ -5602,14 +5603,14 @@ test_dir_should_not_init_request_to_dir_auths_without_v3_info(void *data)
dir_server_add(ds);

directory_get_from_all_authorities(DIR_PURPOSE_FETCH_STATUS_VOTE, 0, NULL);
tt_int_op(CALLED(directory_initiate_request), OP_EQ, 0);
tt_int_op(dir_tests_directory_initiate_request_called, OP_EQ, 0);

directory_get_from_all_authorities(DIR_PURPOSE_FETCH_DETACHED_SIGNATURES, 0,
NULL);
tt_int_op(CALLED(directory_initiate_request), OP_EQ, 0);
tt_int_op(dir_tests_directory_initiate_request_called, OP_EQ, 0);

done:
NS_UNMOCK(directory_initiate_request);
UNMOCK(directory_initiate_request);
clear_dir_servers();
routerlist_free_all();
}
Expand All @@ -5620,7 +5621,8 @@ test_dir_should_init_request_to_dir_auths(void *data)
dir_server_t *ds = NULL;
(void) data;

NS_MOCK(directory_initiate_request);
MOCK(directory_initiate_request,
dir_tests_directory_initiate_request);

clear_dir_servers();
routerlist_free_all();
Expand All @@ -5631,23 +5633,23 @@ test_dir_should_init_request_to_dir_auths(void *data)
dir_server_add(ds);

directory_get_from_all_authorities(DIR_PURPOSE_FETCH_STATUS_VOTE, 0, NULL);
tt_int_op(CALLED(directory_initiate_request), OP_EQ, 1);
tt_int_op(dir_tests_directory_initiate_request_called, OP_EQ, 1);

directory_get_from_all_authorities(DIR_PURPOSE_FETCH_DETACHED_SIGNATURES, 0,
NULL);
tt_int_op(CALLED(directory_initiate_request), OP_EQ, 2);
tt_int_op(dir_tests_directory_initiate_request_called, OP_EQ, 2);

done:
NS_UNMOCK(directory_initiate_request);
UNMOCK(directory_initiate_request);
clear_dir_servers();
routerlist_free_all();
}

void
NS(directory_initiate_request)(directory_request_t *req)
dir_tests_directory_initiate_request(directory_request_t *req)
{
(void)req;
CALLED(directory_initiate_request)++;
dir_tests_directory_initiate_request_called++;
}

static void
Expand Down
Loading

0 comments on commit 5888db4

Please sign in to comment.