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

@@ -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`

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_table = NOBODY;   /* ref to top of table     */
-int top_of_p_file = 0;    /* ref of size of p file   */
+int top_of_p_file = NOBODY;    /* ref of size of p file   */
-long top_idnum = 0;       /* highest idnum in use    */
+long top_idnum = NOBODY;       /* highest idnum in use    */
 /* end previously located in players.c */
 
 struct message_list fight_messages[MAX_MESSAGES];	/* fighting messages	 */
@@ -528,6 +528,7 @@ void destroy_db(void)
   }
 
   /* Rooms */
+  if (top_of_world != NOWHERE) {
     for (cnt = 0; cnt <= top_of_world; cnt++) {
       if (world[cnt].name)
         free(world[cnt].name);
@@ -553,7 +554,7 @@ void destroy_db(void)
       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])
+        if (world[cnt].dir_option[itr] == NULL)
           continue;
 
         if (world[cnt].dir_option[itr]->general_description)
@@ -564,9 +565,11 @@ void destroy_db(void)
       }
     }
     free(world);
-  top_of_world = 0;
+    top_of_world = NOWHERE;
+  }
 
     /* Objects */
+  if (top_of_objt != NOTHING) {
     for (cnt = 0; cnt <= top_of_objt; cnt++) {
       if (obj_proto[cnt].name)
         free(obj_proto[cnt].name);
@@ -583,8 +586,11 @@ void destroy_db(void)
     }
     free(obj_proto);
     free(obj_index);
+    top_of_objt = NOTHING;
+  }
 
   /* Mobiles */
+  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);
@@ -605,7 +611,8 @@ void destroy_db(void)
     }
     free(mob_proto);
     free(mob_index);
-
+    top_of_mobt = NOBODY;
+  }
   /* Shops */
   destroy_shops();
 
@@ -615,6 +622,8 @@ void destroy_db(void)
   /* Zones */
 #define THIS_CMD zone_table[cnt].cmd[itr]
 
+  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);
@@ -634,7 +643,8 @@ void destroy_db(void)
       }
     }
     free(zone_table);
-
+    top_of_zone_table = NOWHERE;
+  }
 #undef THIS_CMD
 
   /* zone table reset queue */
@@ -648,6 +658,8 @@ void destroy_db(void)
   }
 
   /* Triggers */
+  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) */
@@ -668,7 +680,8 @@ void destroy_db(void)
       free(trig_index[cnt]);
     }
     free(trig_index);
-
+    top_of_trigt = NOTHING;
+  }
   /* Events */
   event_free_all();
 

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_MUNIT:= ../munit/munit.o
-OBJ_FILES_FROM_TESTS!= ls *.c | sed 's/\$$.c/.o/g' | xargs
+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)
+testrunner: $(OBJ_FILES)
-	$(CC) $(CFLAGS) -o testfile $(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

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`
+

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->name = strdup("ring");
-    ring1->short_description = "ring1";
+    ring1->short_description = strdup("ring1");
     
     obj_data *ring2 = create_obj();
-    ring2->name = "ring";
+    ring2->name = strdup("ring");
-    ring2->short_description = "ring2";
+    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),
+    EXT_TEST("/do_remove/item_not_found", test_do_remove_should_give_message_on_removing_of_unknown_item, before_each, after_each),
-    STD_TEST("/do_remove/remove_second_item", test_do_remove_should_remove_second_item_by_number),
+    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 }

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 }
+};

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

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;
+}
+

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 <stdarg.h>
+#include <stddef.h>
+#include <setjmp.h>
+#include <cmocka.h>
+
+/*
+ * test fixtures
+ */
+char_data *get_test_char();
+
+void simple_world();
+void add_test_char(room_rnum target_room);
+
+#endif //TEST_FIXTURES_H

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[] = {
   { "/handler.c", handler_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;
 

src/test/testrunner.h

@@ -1,27 +1,7 @@
 #ifndef TESTRUNNER_H
 #define TESTRUNNER_H
 
-#include "../conf.h"
+#include "test.fixtures.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 <stdarg.h>
-#include <stddef.h>
-#include <setjmp.h>
-#include <cmocka.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