Skip to content

Commit

Permalink
(#513) Refactor into set_log_streams_for_tests() to manage unit-test …
Browse files Browse the repository at this point in the history
…outputs.

This commit refactors existing chunks of code that exists in different unit-
test sources to manage output file handles to a common function, now
defined in new file unit/unit_tests_common.c ; set_log_streams_for_tests()
Tests that check error-raising behaviour will now need to call
set_log_streams_for_neg_tests(), to manage output streams.

Minor correction to TEST_DB_NAME; change it to conform to the r.e. defined
in .gitignore to suppress listing this in 'git status' output.
  • Loading branch information
gapisback committed Dec 25, 2022
1 parent bcd7933 commit e2c9a83
Show file tree
Hide file tree
Showing 9 changed files with 107 additions and 59 deletions.
28 changes: 21 additions & 7 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,12 @@ FUNCTIONAL_TESTSRC := $(shell find $(FUNCTIONAL_TESTSDIR) -name "*.c")
# Symbol for all unit-test sources, from which we will build standalone
# unit-test binaries.
UNIT_TESTSRC := $(shell find $(UNIT_TESTSDIR) -name "*.c")

# Specify the r.e. so it will only pick-up sources that are common to multiple
# unit-tests (and won't pick-up test-specific common files, e.g.
# btree_test_common.c)
COMMON_UNIT_TESTSRC := $(shell find $(UNIT_TESTSDIR) -name "*tests_common.c")

TESTSRC := $(COMMON_TESTSRC) $(FUNCTIONAL_TESTSRC) $(UNIT_TESTSRC)

# Some unit-tests will be excluded from the list of dot-oh's that are linked into
Expand Down Expand Up @@ -212,6 +218,9 @@ OBJ := $(SRC:%.c=$(OBJDIR)/%.o)
# Objects from test sources in tests/ that are shared by functional/ and unit/ tests
COMMON_TESTOBJ= $(COMMON_TESTSRC:%.c=$(OBJDIR)/%.o)

# Objects from unit-test sources in tests/unit that are shared by unit tests
COMMON_UNIT_TESTOBJ= $(COMMON_UNIT_TESTSRC:%.c=$(OBJDIR)/%.o)

# Objects from test sources in tests/functional/ sub-dir
FUNCTIONAL_TESTOBJ= $(FUNCTIONAL_TESTSRC:%.c=$(OBJDIR)/%.o)

Expand Down Expand Up @@ -329,7 +338,7 @@ $(LIBDIR)/libsplinterdb.a : $(OBJ) | $$(@D)/. $(CONFIG_FILE)

# Dependencies for the main executables
$(BINDIR)/driver_test: $(FUNCTIONAL_TESTOBJ) $(COMMON_TESTOBJ) $(LIBDIR)/libsplinterdb.so
$(BINDIR)/unit_test: $(FAST_UNIT_TESTOBJS) $(COMMON_TESTOBJ) $(LIBDIR)/libsplinterdb.so $(OBJDIR)/$(FUNCTIONAL_TESTSDIR)/test_async.o
$(BINDIR)/unit_test: $(FAST_UNIT_TESTOBJS) $(COMMON_TESTOBJ) $(COMMON_UNIT_TESTOBJ) $(LIBDIR)/libsplinterdb.so $(OBJDIR)/$(FUNCTIONAL_TESTSDIR)/test_async.o

#################################################################
# Dependencies for the mini unit tests
Expand Down Expand Up @@ -386,7 +395,7 @@ BTREE_SYS = $(OBJDIR)/$(SRCDIR)/btree.o \
# Note each test bin/unit/<x> also depends on obj/unit/<x>.o, as
# defined above using unit_test_self_dependency.
#
$(BINDIR)/$(UNITDIR)/misc_test: $(UTIL_SYS)
$(BINDIR)/$(UNITDIR)/misc_test: $(UTIL_SYS) $(COMMON_UNIT_TESTOBJ)

$(BINDIR)/$(UNITDIR)/util_test: $(UTIL_SYS)

Expand All @@ -401,20 +410,24 @@ $(BINDIR)/$(UNITDIR)/btree_stress_test: $(OBJDIR)/$(UNIT_TESTSDIR)/btree_test_co
$(BTREE_SYS)

$(BINDIR)/$(UNITDIR)/splinter_test: $(COMMON_TESTOBJ) \
$(COMMON_UNIT_TESTOBJ) \
$(OBJDIR)/$(FUNCTIONAL_TESTSDIR)/test_async.o \
$(LIBDIR)/libsplinterdb.so

$(BINDIR)/$(UNITDIR)/splinterdb_quick_test: $(COMMON_TESTOBJ) \
$(OBJDIR)/$(FUNCTIONAL_TESTSDIR)/test_async.o \
$(LIBDIR)/libsplinterdb.so
$(COMMON_UNIT_TESTOBJ) \
$(OBJDIR)/$(FUNCTIONAL_TESTSDIR)/test_async.o \
$(LIBDIR)/libsplinterdb.so

$(BINDIR)/$(UNITDIR)/splinterdb_stress_test: $(COMMON_TESTOBJ) \
$(OBJDIR)/$(FUNCTIONAL_TESTSDIR)/test_async.o \
$(LIBDIR)/libsplinterdb.so
$(COMMON_UNIT_TESTOBJ) \
$(OBJDIR)/$(FUNCTIONAL_TESTSDIR)/test_async.o \
$(LIBDIR)/libsplinterdb.so

$(BINDIR)/$(UNITDIR)/writable_buffer_test: $(UTIL_SYS)

$(BINDIR)/$(UNITDIR)/limitations_test: $(COMMON_TESTOBJ) \
$(BINDIR)/$(UNITDIR)/limitations_test: $(COMMON_TESTOBJ) \
$(COMMON_UNIT_TESTOBJ) \
$(OBJDIR)/$(FUNCTIONAL_TESTSDIR)/test_async.o \
$(LIBDIR)/libsplinterdb.so

Expand All @@ -425,6 +438,7 @@ $(BINDIR)/$(UNITDIR)/config_parse_test: $(UTIL_SYS)

$(BINDIR)/$(UNITDIR)/task_system_test: $(UTIL_SYS) \
$(COMMON_TESTOBJ) \
$(COMMON_UNIT_TESTOBJ) \
$(OBJDIR)/$(FUNCTIONAL_TESTSDIR)/test_async.o \
$(LIBDIR)/libsplinterdb.so

Expand Down
16 changes: 1 addition & 15 deletions tests/unit/limitations_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -63,21 +63,7 @@ CTEST_DATA(limitations)
*/
CTEST_SETUP(limitations)
{
// This test exercises error cases, so even when everything succeeds
// it generates lots of "error" messages.
// By default, that would go to stderr, which would pollute test output.
// Here we ensure those expected error messages are only printed
// when the caller sets the VERBOSE env var to opt-in.
if (Ctest_verbose) {
platform_set_log_streams(stdout, stderr);
CTEST_LOG_INFO("\nVerbose mode on. This test exercises an error case, "
"so on sucess it "
"will print a message that appears to be an error.\n");
} else {
FILE *dev_null = fopen("/dev/null", "w");
ASSERT_NOT_NULL(dev_null);
platform_set_log_streams(dev_null, dev_null);
}
set_log_streams_for_neg_tests(NULL, NULL);

uint64 heap_capacity = (1 * GiB);

Expand Down
14 changes: 2 additions & 12 deletions tests/unit/misc_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -58,22 +58,12 @@ test_vfprintf_usermsg(platform_log_handle *log_handle,
*/
CTEST_DATA(misc)
{
FILE *log_output;
platform_log_handle *log_output;
};

CTEST_SETUP(misc)
{
// It can be surprising to see errors printed from a successful test run.
// So lets send those messages to /dev/null unless VERBOSE=1.
if (Ctest_verbose) {
data->log_output = stdout;
CTEST_LOG_INFO("\nVerbose mode on. This test exercises error-reporting "
"logic, so on success it will print a message "
"that appears to be an error.\n");
} else {
data->log_output = fopen("/dev/null", "w");
ASSERT_NOT_NULL(data->log_output);
}
set_log_streams_for_neg_tests(&data->log_output, NULL);
}

// Optional teardown function for suite, called after every test in suite
Expand Down
5 changes: 2 additions & 3 deletions tests/unit/splinter_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,8 @@ CTEST_DATA(splinter)
// clang-format off
CTEST_SETUP(splinter)
{
if (Ctest_verbose) {
platform_set_log_streams(stdout, stderr);
}
set_log_streams_for_tests();

// Defaults: For basic unit-tests, use single threads
data->num_insert_threads = 1;
data->num_lookup_threads = 1;
Expand Down
4 changes: 1 addition & 3 deletions tests/unit/splinterdb_quick_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,7 @@ CTEST_DATA(splinterdb_quick)
// Optional setup function for suite, called before every test in suite
CTEST_SETUP(splinterdb_quick)
{
if (Ctest_verbose) {
platform_set_log_streams(stdout, stderr);
}
set_log_streams_for_tests();

default_data_config_init(TEST_MAX_KEY_SIZE, &data->default_data_cfg.super);
create_default_cfg(&data->cfg, &data->default_data_cfg.super);
Expand Down
4 changes: 1 addition & 3 deletions tests/unit/splinterdb_stress_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,7 @@ CTEST_DATA(splinterdb_stress)
// Setup function for suite, called before every test in suite
CTEST_SETUP(splinterdb_stress)
{
if (Ctest_verbose) {
platform_set_log_streams(stdout, stderr);
}
set_log_streams_for_tests();

data->cfg = (splinterdb_config){.filename = TEST_DB_NAME,
.cache_size = 1000 * Mega,
Expand Down
16 changes: 1 addition & 15 deletions tests/unit/task_system_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -105,21 +105,7 @@ CTEST_DATA(task_system)
CTEST_SETUP(task_system)
{
platform_status rc = STATUS_OK;
// This test exercises error cases, so even when everything succeeds
// it generates lots of "error" messages.
// By default, that would go to stderr, which would pollute test output.
// Here we ensure those expected error messages are only printed
// when the caller sets the VERBOSE env var to opt-in.
if (Ctest_verbose) {
platform_set_log_streams(stdout, stderr);
CTEST_LOG_INFO("\nVerbose mode on. This test exercises an error case, "
"so on sucess it "
"will print a message that appears to be an error.\n");
} else {
FILE *dev_null = fopen("/dev/null", "w");
ASSERT_NOT_NULL(dev_null);
platform_set_log_streams(dev_null, dev_null);
}
set_log_streams_for_neg_tests(NULL, NULL);

uint64 heap_capacity = (256 * MiB); // small heap is sufficient.
// Create a heap for io and task system to use.
Expand Down
11 changes: 10 additions & 1 deletion tests/unit/unit_tests.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,18 @@
* Define things in here that are going to be needed across most unit tests.
*/

#include "splinterdb/public_platform.h"

/* Name of SplinterDB device created for unit-tests */
#define TEST_DB_NAME "unit_tests_db"
#define TEST_DB_NAME "splinterdb_unit_tests_db"

#define Kilo (1024UL)
#define Mega (1024UL * Kilo)
#define Giga (1024UL * Mega)

void
set_log_streams_for_tests();

void
set_log_streams_for_neg_tests(platform_log_handle **log_stdout,
platform_log_handle **log_stderr);
68 changes: 68 additions & 0 deletions tests/unit/unit_tests_common.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
// Copyright 2022 VMware, Inc.
// SPDX-License-Identifier: Apache-2.0

/*
* -----------------------------------------------------------------------------
* unit_tests_common.c
*
* Common functions shared across unit test sources.
* -----------------------------------------------------------------------------
*/
#include "ctest.h" // This is required for all test-case files.
#include "unit_tests.h"

/*
* Setup function to manage manage output log streams from most (unit) tests.
* By default, there will be no output. Info / error messages are only printed
* when the caller sets the VERBOSE env var to opt-in.
*/
void
set_log_streams_for_tests()
{
if (Ctest_verbose) {
platform_set_log_streams(stdout, stderr);
}
}

/*
* Setup function is provided to manage output log streams from (unit) tests.
* that are negative tests, checking error conditions / messages.
*
* Some unit tests exercise error cases, so even when everything succeeds, they
* generate lots of "error" messages. By default, that would go to stderr, which
* would pollute test output. This function sets up output file handles such
* that those expected error messages are only printed when the caller sets the
* VERBOSE env var to opt-in. By default, execution from unit-tests will be
* silent, redirecting output handles to /dev/null.
*
* Returns: Output file handles (out/error) if caller has requested for them.
*/
void
set_log_streams_for_neg_tests(platform_log_handle **log_stdout,
platform_log_handle **log_stderr)
{
// Here we ensure those expected error messages are only printed
// when the caller sets the VERBOSE env var to opt-in.
if (Ctest_verbose) {
platform_set_log_streams(stdout, stderr);
if (log_stdout) {
*log_stdout = stdout;
}
if (log_stderr) {
*log_stderr = stderr;
}
CTEST_LOG_INFO("\nVerbose mode on. This test exercises an error case, "
"so on sucess it "
"will print a message that appears to be an error.\n");
} else {
FILE *dev_null = fopen("/dev/null", "w");
ASSERT_NOT_NULL(dev_null);
platform_set_log_streams(dev_null, dev_null);
if (log_stdout) {
*log_stdout = dev_null;
}
if (log_stderr) {
*log_stderr = dev_null;
}
}
}

0 comments on commit e2c9a83

Please sign in to comment.