From c8fc70bf43f931813be8f927a7366f39db09da7e Mon Sep 17 00:00:00 2001 From: welcor <357770+welcor@users.noreply.github.com> Date: Thu, 27 Jun 2024 00:31:54 +0200 Subject: [PATCH] Refactor: fixtures in its own files. Added readme for creating tests. Found and fixed interesting bugs in destroy_db when there is no world. renamed the testfile to testrunner to make it clear it is actually running the texts. Also, made testrunner more focused on the actual running of the tests. Added debug target to test makefile. --- README.md | 6 +- src/db.c | 223 +++++++++++++++++++++------------------ src/test/Makefile | 24 +++-- src/test/README.md | 47 +++++++++ src/test/test.act.item.c | 22 ++-- src/test/test.example.c | 14 +++ src/test/test.example.h | 10 ++ src/test/test.fixtures.c | 46 ++++++++ src/test/test.fixtures.h | 34 ++++++ src/test/testrunner.c | 41 +------ src/test/testrunner.h | 32 ++---- 11 files changed, 318 insertions(+), 181 deletions(-) create mode 100644 src/test/README.md create mode 100644 src/test/test.example.c create mode 100644 src/test/test.example.h create mode 100644 src/test/test.fixtures.c create mode 100644 src/test/test.fixtures.h diff --git a/README.md b/README.md index dd40ea1..a934ab0 100644 --- a/README.md +++ b/README.md @@ -13,5 +13,9 @@ Read more in the doc/ folder ## To run the tests -1. clone the munit library into src/munit. It is registered as a submodule in git `git submodule init` +1. clone the munit library into src/munit. It is registered as a submodule in git + +`git submodule init && git submodule update` + 2. install the cmocka-library: `sudo apt install libcmocka-dev` +3. `./config.status && cd src && make test` \ No newline at end of file diff --git a/src/db.c b/src/db.c index fda7ac6..b06de45 100644 --- a/src/db.c +++ b/src/db.c @@ -44,26 +44,26 @@ struct config_data config_info; /* Game configuration list. */ struct room_data *world = NULL; /* array of rooms */ -room_rnum top_of_world = 0; /* ref to top element of world */ +room_rnum top_of_world = NOWHERE; /* ref to top element of world */ struct char_data *character_list = NULL; /* global linked list of chars */ struct index_data *mob_index; /* index table for mobile file */ struct char_data *mob_proto; /* prototypes for mobs */ -mob_rnum top_of_mobt = 0; /* top of mobile index table */ +mob_rnum top_of_mobt = NOBODY; /* top of mobile index table */ struct obj_data *object_list = NULL; /* global linked list of objs */ struct index_data *obj_index; /* index table for object file */ struct obj_data *obj_proto; /* prototypes for objs */ -obj_rnum top_of_objt = 0; /* top of object index table */ +obj_rnum top_of_objt = NOTHING; /* top of object index table */ struct zone_data *zone_table; /* zone table */ -zone_rnum top_of_zone_table = 0;/* top element of zone tab */ +zone_rnum top_of_zone_table = NOTHING;/* top element of zone tab */ /* begin previously located in players.c */ struct player_index_element *player_table = NULL; /* index to plr file */ -int top_of_p_table = 0; /* ref to top of table */ -int top_of_p_file = 0; /* ref of size of p file */ -long top_idnum = 0; /* highest idnum in use */ +int top_of_p_table = NOBODY; /* ref to top of table */ +int top_of_p_file = NOBODY; /* ref of size of p file */ +long top_idnum = NOBODY; /* highest idnum in use */ /* end previously located in players.c */ struct message_list fight_messages[MAX_MESSAGES]; /* fighting messages */ @@ -528,84 +528,91 @@ void destroy_db(void) } /* Rooms */ - for (cnt = 0; cnt <= top_of_world; cnt++) { - if (world[cnt].name) - free(world[cnt].name); - if (world[cnt].description) - free(world[cnt].description); - free_extra_descriptions(world[cnt].ex_description); + if (top_of_world != NOWHERE) { + for (cnt = 0; cnt <= top_of_world; cnt++) { + if (world[cnt].name) + free(world[cnt].name); + if (world[cnt].description) + free(world[cnt].description); + free_extra_descriptions(world[cnt].ex_description); - if (world[cnt].events != NULL) { - if (world[cnt].events->iSize > 0) { - struct event * pEvent; + if (world[cnt].events != NULL) { + if (world[cnt].events->iSize > 0) { + struct event * pEvent; - while ((pEvent = simple_list(world[cnt].events)) != NULL) - event_cancel(pEvent); - } - free_list(world[cnt].events); - world[cnt].events = NULL; - } + while ((pEvent = simple_list(world[cnt].events)) != NULL) + event_cancel(pEvent); + } + free_list(world[cnt].events); + world[cnt].events = NULL; + } - /* free any assigned scripts */ - if (SCRIPT(&world[cnt])) - extract_script(&world[cnt], WLD_TRIGGER); - /* free script proto list */ - free_proto_script(&world[cnt], WLD_TRIGGER); + /* free any assigned scripts */ + if (SCRIPT(&world[cnt])) + extract_script(&world[cnt], WLD_TRIGGER); + /* free script proto list */ + free_proto_script(&world[cnt], WLD_TRIGGER); - for (itr = 0; itr < NUM_OF_DIRS; itr++) { /* NUM_OF_DIRS here, not DIR_COUNT */ - if (!world[cnt].dir_option[itr]) - continue; + for (itr = 0; itr < NUM_OF_DIRS; itr++) { /* NUM_OF_DIRS here, not DIR_COUNT */ + if (world[cnt].dir_option[itr] == NULL) + continue; - if (world[cnt].dir_option[itr]->general_description) - free(world[cnt].dir_option[itr]->general_description); - if (world[cnt].dir_option[itr]->keyword) - free(world[cnt].dir_option[itr]->keyword); - free(world[cnt].dir_option[itr]); + if (world[cnt].dir_option[itr]->general_description) + free(world[cnt].dir_option[itr]->general_description); + if (world[cnt].dir_option[itr]->keyword) + free(world[cnt].dir_option[itr]->keyword); + free(world[cnt].dir_option[itr]); + } } + free(world); + top_of_world = NOWHERE; } - free(world); - top_of_world = 0; - /* Objects */ - for (cnt = 0; cnt <= top_of_objt; cnt++) { - if (obj_proto[cnt].name) - free(obj_proto[cnt].name); - if (obj_proto[cnt].description) - free(obj_proto[cnt].description); - if (obj_proto[cnt].short_description) - free(obj_proto[cnt].short_description); - if (obj_proto[cnt].action_description) - free(obj_proto[cnt].action_description); - free_extra_descriptions(obj_proto[cnt].ex_description); + /* Objects */ + if (top_of_objt != NOTHING) { + for (cnt = 0; cnt <= top_of_objt; cnt++) { + if (obj_proto[cnt].name) + free(obj_proto[cnt].name); + if (obj_proto[cnt].description) + free(obj_proto[cnt].description); + if (obj_proto[cnt].short_description) + free(obj_proto[cnt].short_description); + if (obj_proto[cnt].action_description) + free(obj_proto[cnt].action_description); + free_extra_descriptions(obj_proto[cnt].ex_description); - /* free script proto list */ - free_proto_script(&obj_proto[cnt], OBJ_TRIGGER); + /* free script proto list */ + free_proto_script(&obj_proto[cnt], OBJ_TRIGGER); + } + free(obj_proto); + free(obj_index); + top_of_objt = NOTHING; } - free(obj_proto); - free(obj_index); /* Mobiles */ - for (cnt = 0; cnt <= top_of_mobt; cnt++) { - if (mob_proto[cnt].player.name) - free(mob_proto[cnt].player.name); - if (mob_proto[cnt].player.title) - free(mob_proto[cnt].player.title); - if (mob_proto[cnt].player.short_descr) - free(mob_proto[cnt].player.short_descr); - if (mob_proto[cnt].player.long_descr) - free(mob_proto[cnt].player.long_descr); - if (mob_proto[cnt].player.description) - free(mob_proto[cnt].player.description); + if (top_of_mobt != NOBODY) { + for (cnt = 0; cnt <= top_of_mobt; cnt++) { + if (mob_proto[cnt].player.name) + free(mob_proto[cnt].player.name); + if (mob_proto[cnt].player.title) + free(mob_proto[cnt].player.title); + if (mob_proto[cnt].player.short_descr) + free(mob_proto[cnt].player.short_descr); + if (mob_proto[cnt].player.long_descr) + free(mob_proto[cnt].player.long_descr); + if (mob_proto[cnt].player.description) + free(mob_proto[cnt].player.description); - /* free script proto list */ - free_proto_script(&mob_proto[cnt], MOB_TRIGGER); + /* free script proto list */ + free_proto_script(&mob_proto[cnt], MOB_TRIGGER); - while (mob_proto[cnt].affected) - affect_remove(&mob_proto[cnt], mob_proto[cnt].affected); + while (mob_proto[cnt].affected) + affect_remove(&mob_proto[cnt], mob_proto[cnt].affected); + } + free(mob_proto); + free(mob_index); + top_of_mobt = NOBODY; } - free(mob_proto); - free(mob_index); - /* Shops */ destroy_shops(); @@ -615,26 +622,29 @@ void destroy_db(void) /* Zones */ #define THIS_CMD zone_table[cnt].cmd[itr] - for (cnt = 0; cnt <= top_of_zone_table; cnt++) { - if (zone_table[cnt].name) - free(zone_table[cnt].name); - if (zone_table[cnt].builders) - free(zone_table[cnt].builders); - if (zone_table[cnt].cmd) { - /* first see if any vars were defined in this zone */ - for (itr = 0;THIS_CMD.command != 'S';itr++) - if (THIS_CMD.command == 'V') { - if (THIS_CMD.sarg1) - free(THIS_CMD.sarg1); - if (THIS_CMD.sarg2) - free(THIS_CMD.sarg2); - } - /* then free the command list */ - free(zone_table[cnt].cmd); + if (top_of_zone_table != NOWHERE) + { + for (cnt = 0; cnt <= top_of_zone_table; cnt++) { + if (zone_table[cnt].name) + free(zone_table[cnt].name); + if (zone_table[cnt].builders) + free(zone_table[cnt].builders); + if (zone_table[cnt].cmd) { + /* first see if any vars were defined in this zone */ + for (itr = 0;THIS_CMD.command != 'S';itr++) + if (THIS_CMD.command == 'V') { + if (THIS_CMD.sarg1) + free(THIS_CMD.sarg1); + if (THIS_CMD.sarg2) + free(THIS_CMD.sarg2); + } + /* then free the command list */ + free(zone_table[cnt].cmd); + } } + free(zone_table); + top_of_zone_table = NOWHERE; } - free(zone_table); - #undef THIS_CMD /* zone table reset queue */ @@ -648,27 +658,30 @@ void destroy_db(void) } /* Triggers */ - for (cnt=0; cnt < top_of_trigt; cnt++) { - if (trig_index[cnt]->proto) { - /* make sure to nuke the command list (memory leak) */ - /* free_trigger() doesn't free the command list */ - if (trig_index[cnt]->proto->cmdlist) { - struct cmdlist_element *i, *j; - i = trig_index[cnt]->proto->cmdlist; - while (i) { - j = i->next; - if (i->cmd) - free(i->cmd); - free(i); - i = j; + if (top_of_trigt != NOTHING) + { + for (cnt=0; cnt < top_of_trigt; cnt++) { + if (trig_index[cnt]->proto) { + /* make sure to nuke the command list (memory leak) */ + /* free_trigger() doesn't free the command list */ + if (trig_index[cnt]->proto->cmdlist) { + struct cmdlist_element *i, *j; + i = trig_index[cnt]->proto->cmdlist; + while (i) { + j = i->next; + if (i->cmd) + free(i->cmd); + free(i); + i = j; + } } + free_trigger(trig_index[cnt]->proto); } - free_trigger(trig_index[cnt]->proto); + free(trig_index[cnt]); } - free(trig_index[cnt]); + free(trig_index); + top_of_trigt = NOTHING; } - free(trig_index); - /* Events */ event_free_all(); diff --git a/src/test/Makefile b/src/test/Makefile index c3d1c72..9036ef4 100644 --- a/src/test/Makefile +++ b/src/test/Makefile @@ -8,7 +8,7 @@ ASAN:=n UBSAN:=n EXTENSION:= TEST_ENV:= -CFLAGS:=-Wall -Wno-char-subscripts -Wno-unused-but-set-variable +CFLAGS:=-Wall -Wno-char-subscripts -Wno-unused-but-set-variable -g CFLAGS+=-Wl,--wrap=send_to_char,--wrap=vwrite_to_output AGGRESSIVE_WARNINGS=n LIBS:=-lcrypt @@ -45,27 +45,31 @@ ifneq ($(CC),pgcc) endif endif -MUNIT_FILES := ../munit/munit.h ../munit/munit.c +#MUNIT_FILES := ../munit/munit.h ../munit/munit.c TEST_FILES := $(ls *.c) # exclude main.c to avoid having multiple entrypoints. OBJ_FILES_FROM_MUD_CODE != ls ../*.c | grep -v main.c | sed 's/\.c$$/.o/g' | xargs -OBJ_FILES_FORM_MUNIT:= ../munit/munit.o -OBJ_FILES_FROM_TESTS!= ls *.c | sed 's/\$$.c/.o/g' | xargs +OBJ_FILES_FROM_MUNIT:= ../munit/munit.o +OBJ_FILES_FROM_TESTS!= ls *.c | sed 's/\.c/.o/g' | xargs -OBJ_FILES := $(OBJ_FILES_FROM_MUD_CODE) $(OBJ_FILES_FORM_MUNIT) $(OBJ_FILES_FROM_TESTS) $(LIBS) +OBJ_FILES := $(OBJ_FILES_FROM_MUD_CODE) $(OBJ_FILES_FROM_MUNIT) $(OBJ_FILES_FROM_TESTS) $(LIBS) -testfile: $(OBJ_FILES) - $(CC) $(CFLAGS) -o testfile $(OBJ_FILES) +testrunner: $(OBJ_FILES) + $(CC) $(CFLAGS) -o testrunner $(OBJ_FILES) + +test: testrunner + $(TEST_ENV) ./testrunner + +debug: testrunner + $(TEST_ENV) gdb -q --args ./testrunner --no-fork -test: testfile - $(TEST_ENV) ./testfile $%.o: %.c $(CC) $< $(CFLAGS) -c -o $@ clean: - rm -f *.o testfile depend + rm -f *.o testrunner depend all: test default: all diff --git a/src/test/README.md b/src/test/README.md new file mode 100644 index 0000000..d84ca22 --- /dev/null +++ b/src/test/README.md @@ -0,0 +1,47 @@ +# unit and integration tests for tbamud + +## how do I add a new test? +Open the .c file of your choosing and add a `UNIT_TEST` function. +The function will have access to all the global variables and all non-static +functions in the code, but there will be no data loaded. + +The name of the function will be listed when the tests are run. + + + + +The [munit website](https://nemequ.github.io/munit/#getting-started) may be useful for more details. + + +## how do I add new files with tests? +First, create your test file. As a general rule, keep unit tests in files named + after the files containing the functions you are testing. For instance, if you're + testing the `do_simple_move()` function, create a file called `test.act.movement.c`. + +You can use the example file `test.example.c` as a template. +The `.c`-file needs a couple of boilerplate parts: + +- An import statement to include the `.h`-file. +- UNIT_TEST-functions. See above. +- An array of `MunitTest`s for inclusion in the runner app. + The name in these are concatenated between the name in testrunner and the name of the tests in the output. + This is useful for grouping. + +Next, create a header file for your tests. It's a good idea to keep the same name, + with a .h postfix. So in the example, it'll be `test.act.movement.h`. + +You can use the `test.example.h` file as a template. It needs a little boilerplate, too. + +- It needs to include the testrunner.h for the prototype of UNIT_TEST and access to munit-structs. +- It needs a guard to only be loaded once (the `#ifndef/#define/#endif` incantation at the start and end) +- It needs a prototype of all tests in the .c-file. +- It needs a prototype of the array of tests. + +Finally, add the array to the suites array in `testrunner.c` to actually run the tests. + +- Add the .h file to the list of imported files. +- Add a row to the suites array. The name in this list is prepended to every test in the given + file when listing the results. + +Now, having all the bits and pieces ready, you can add you unit tests, and run them with `make test` + diff --git a/src/test/test.act.item.c b/src/test/test.act.item.c index 193dcb3..0e13120 100644 --- a/src/test/test.act.item.c +++ b/src/test/test.act.item.c @@ -15,12 +15,12 @@ UNIT_TEST(test_do_remove_should_remove_second_item_by_number) { char_data *ch = get_test_char(); obj_data *ring1 = create_obj(); - ring1->name = "ring"; - ring1->short_description = "ring1"; + ring1->name = strdup("ring"); + ring1->short_description = strdup("ring1"); obj_data *ring2 = create_obj(); - ring2->name = "ring"; - ring2->short_description = "ring2"; + ring2->name = strdup("ring"); + ring2->short_description = strdup("ring2"); equip_char(ch, ring1, WEAR_FINGER_R); equip_char(ch, ring2, WEAR_FINGER_L); @@ -33,9 +33,19 @@ UNIT_TEST(test_do_remove_should_remove_second_item_by_number) { return MUNIT_OK; } +static void* before_each(const MunitParameter params[], void* user_data) { + simple_world(); + add_test_char(0); + return NULL; +} + +static void after_each(void* fixture) { + destroy_db(); +} + MunitTest act_item_c_tests[] = { - STD_TEST("/do_remove/item_not_found", test_do_remove_should_give_message_on_removing_of_unknown_item), - STD_TEST("/do_remove/remove_second_item", test_do_remove_should_remove_second_item_by_number), + EXT_TEST("/do_remove/item_not_found", test_do_remove_should_give_message_on_removing_of_unknown_item, before_each, after_each), + EXT_TEST("/do_remove/remove_second_item", test_do_remove_should_remove_second_item_by_number, before_each, after_each), // end of array marker { NULL, NULL, NULL, NULL, MUNIT_TEST_OPTION_NONE, NULL } diff --git a/src/test/test.example.c b/src/test/test.example.c new file mode 100644 index 0000000..13b9043 --- /dev/null +++ b/src/test/test.example.c @@ -0,0 +1,14 @@ +#include "test.example.h" + +UNIT_TEST(example_test) +{ + return MUNIT_OK; +} + + +MunitTest test_example_c_tests[] = { + STD_TEST("/example/example_test", example_test), + + // end of array marker + { NULL, NULL, NULL, NULL, MUNIT_TEST_OPTION_NONE, NULL } +}; diff --git a/src/test/test.example.h b/src/test/test.example.h new file mode 100644 index 0000000..4e4a69a --- /dev/null +++ b/src/test/test.example.h @@ -0,0 +1,10 @@ +#include "testrunner.h" + +#ifndef TEST_EXAMPLE_H +#define TEST_EXAMPLE_H + +extern MunitTest test_example_c_tests[]; + +UNIT_TEST(example_test); + +#endif \ No newline at end of file diff --git a/src/test/test.fixtures.c b/src/test/test.fixtures.c new file mode 100644 index 0000000..ec03833 --- /dev/null +++ b/src/test/test.fixtures.c @@ -0,0 +1,46 @@ +#include "test.fixtures.h" + + +/* + * test-fixtures common for many tests +*/ + +static char_data* test_char; + +void simple_world() +{ + int i; + CREATE(world, struct room_data, 1); + top_of_world = 0; + world[0].func = NULL; + world[0].contents = NULL; + world[0].people = NULL; + world[0].light = 0; + SCRIPT(&world[0]) = NULL; + + for (i = 0; i < NUM_OF_DIRS; i++) + world[0].dir_option[i] = NULL; + + world[0].ex_description = NULL; +} + + +char_data *get_test_char() { + return test_char; +} + +void add_test_char(room_rnum target_room_rnum) +{ + if (top_of_world < 0) { + fprintf(stderr, "World not created, nowhere to put character in add_test_char"); + exit(-1); + } + char_data *ch = create_char(); + CREATE(ch->player_specials, struct player_special_data, 1); + ch->char_specials.position = POS_STANDING; + CREATE(ch->desc, struct descriptor_data, 1); + + char_to_room(ch, target_room_rnum); + test_char = ch; +} + diff --git a/src/test/test.fixtures.h b/src/test/test.fixtures.h new file mode 100644 index 0000000..5f40766 --- /dev/null +++ b/src/test/test.fixtures.h @@ -0,0 +1,34 @@ +#ifndef TEST_FIXTURES_H +#define TEST_FIXTURES_H + +#include "../conf.h" +#include "../sysdep.h" +#include "../structs.h" +#include "../utils.h" +#include "../comm.h" +#include "../db.h" +#include "../handler.h" +#include "../screen.h" +#include "../interpreter.h" +#include "../spells.h" +#include "../dg_scripts.h" +#include "../act.h" +#include "../class.h" +#include "../fight.h" +#include "../quest.h" +#include "../mud_event.h" +#include "../munit/munit.h" +#include +#include +#include +#include + +/* + * test fixtures + */ +char_data *get_test_char(); + +void simple_world(); +void add_test_char(room_rnum target_room); + +#endif //TEST_FIXTURES_H diff --git a/src/test/testrunner.c b/src/test/testrunner.c index 18bafe3..2b1a1f2 100644 --- a/src/test/testrunner.c +++ b/src/test/testrunner.c @@ -1,15 +1,13 @@ #include "testrunner.h" #include "test.handler.h" #include "test.act.item.h" +#include "test.example.h" -static void simple_world(); -static void add_char(); - -static char_data* test_char; - -static MunitSuite suites[] = { +static MunitSuite suites[] = { { "/handler.c", handler_c_tests, NULL, 1, MUNIT_SUITE_OPTION_NONE }, - { "/act.item.c", act_item_c_tests, NULL, 1, MUNIT_SUITE_OPTION_NONE }, + { "/act.item.c", act_item_c_tests, NULL, 1, MUNIT_SUITE_OPTION_NONE }, + { "/test.example.c", test_example_c_tests, NULL, 1, MUNIT_SUITE_OPTION_NONE }, + { NULL, NULL, NULL, 0, MUNIT_SUITE_OPTION_NONE } }; @@ -24,39 +22,10 @@ static const MunitSuite test_suite = { int main(int argc, char* argv[MUNIT_ARRAY_PARAM(argc + 1)]) { logfile = stderr; - simple_world(); - add_char(); return munit_suite_main(&test_suite, (void*) "µnit", argc, argv); } -/* - * test-fixtures common for many tests -*/ - -static void simple_world() -{ - CREATE(world, struct room_data, 1); - top_of_world = 1; -} - -char_data *get_test_char() { - return test_char; -} - -static void add_char() -{ - char_data *ch = create_char(); - CREATE(ch->player_specials, struct player_special_data, 1); - new_mobile_data(ch); - ch->char_specials.position = POS_STANDING; - CREATE(ch->desc, struct descriptor_data, 1); - - char_to_room(ch, 0); - test_char = ch; -} - - static char testbuf[MAX_OUTPUT_BUFFER]; static int testbuf_size = 0; diff --git a/src/test/testrunner.h b/src/test/testrunner.h index 68eafe0..9613d7b 100644 --- a/src/test/testrunner.h +++ b/src/test/testrunner.h @@ -1,27 +1,7 @@ #ifndef TESTRUNNER_H #define TESTRUNNER_H -#include "../conf.h" -#include "../sysdep.h" -#include "../structs.h" -#include "../utils.h" -#include "../comm.h" -#include "../db.h" -#include "../handler.h" -#include "../screen.h" -#include "../interpreter.h" -#include "../spells.h" -#include "../dg_scripts.h" -#include "../act.h" -#include "../class.h" -#include "../fight.h" -#include "../quest.h" -#include "../mud_event.h" -#include "../munit/munit.h" -#include -#include -#include -#include +#include "test.fixtures.h" /** * Utility macro for defining tests. @@ -35,9 +15,15 @@ #define STD_TEST(test_name, test_fun) { (char *)(test_name), (test_fun), NULL, NULL, MUNIT_TEST_OPTION_NONE, NULL } /* - * test fixtures + * An "extended test" has setup or teardown but doesn't take any parameters. + * This is a utility macro for the test suite listing. +*/ +#define EXT_TEST(test_name, test_fun, setup_fun, teardown_fun) { (char *)(test_name), (test_fun), (setup_fun), (teardown_fun), MUNIT_TEST_OPTION_NONE, NULL } + + +/* + * Returns the latest messages sent through send_to_char() or act() */ -char_data *get_test_char(); char *get_last_messages(); #endif \ No newline at end of file