[PATCH v4 1/6] Added simple unit/integration test framework and corresponding test program.

Jon A. Cruz jonc at osg.samsung.com
Tue Jun 23 21:55:41 PDT 2015


On 06/22/2015 07:19 AM, Pekka Paalanen wrote:
> Hi Jon,
> 
> this is just a quick casual look, so I'm only mentioning what cought my
> eye but mostly ignoring minor issues like style.
> 
> I know you sent a v5 series, but it sounded like my comments here are
> still applicable.
> 
> I do get kind of confused every time a function is used earlier in the
> file than where it is defined. I have a habit of scrolling up when I
> see a function call I want to know what it does. Is that just me?

Looking into it more, I think I've figured out much of that on my part
comes from C++ and other OO languages I've worked in. Especially with
C++ one should start a class declaration with a single 'public' section
followed optionally by a single 'protected' section and finally a single
'private' section if needed. So the general structure tends to be from
higher-level first in the file down to lower-level later on. Also IDE
use can promote the "F3 to go do declaration" over manually scrolling
approach.

Since weston here is solidly in the "non-C++ C camp" I'll be reworking
the order to match (either in later revisions or in follow-ups as needed).


> More below.
> 
> 
> On Thu, 11 Jun 2015 22:01:27 -0700
> "Jon A. Cruz" <jonc at osg.samsung.com> wrote:
> 
>> Added a simple C-based test framework and an example program
>> that uses it to run through some simple wayland client checks.
>>
>> This is new code inspired primarily by the approaches of Google
>> Test, Boost Test, JUnit and TestNG. Factors of others were also
>> considered during design and implementation.
>>
>> Signed-off-by: Jon A. Cruz <jonc at osg.samsung.com>
>> ---
>>  .gitignore                                 |    3 +
>>  Makefile.am                                |   87 +-
>>  tools/waycheck/moretest.c                  |   89 ++
>>  tools/waycheck/rough_draw.c                |  273 +++++
>>  tools/waycheck/rough_draw.h                |   62 +
>>  tools/waycheck/waycheck.c                  |  435 +++++++
>>  tools/waycheck/waycheck.dox                |   29 +
>>  tools/wayland_fixtures/inc/wtst_fixtures.h |  279 +++++
>>  tools/wayland_fixtures/src/wtst_fixtures.c | 1115 ++++++++++++++++++
>>  tools/zunitc/doc/zunitc.dox                |  138 +++
>>  tools/zunitc/inc/zunitc/zunitc.h           |  681 +++++++++++
>>  tools/zunitc/inc/zunitc/zunitc_impl.h      |  102 ++
>>  tools/zunitc/src/main.c                    |   46 +
>>  tools/zunitc/src/zuc_base_logger.c         |  401 +++++++
>>  tools/zunitc/src/zuc_base_logger.h         |   35 +
>>  tools/zunitc/src/zuc_collector.c           |  424 +++++++
>>  tools/zunitc/src/zuc_collector.h           |   55 +
>>  tools/zunitc/src/zuc_context.h             |   55 +
>>  tools/zunitc/src/zuc_event.h               |   83 ++
>>  tools/zunitc/src/zuc_event_listener.h      |  171 +++
>>  tools/zunitc/src/zuc_types.h               |   77 ++
>>  tools/zunitc/src/zunitc_impl.c             | 1726 ++++++++++++++++++++++++++++
>>  tools/zunitc/test/fixtures_test.c          |  104 ++
>>  tools/zunitc/test/zunitc_test.c            |  456 ++++++++
>>  24 files changed, 6925 insertions(+), 1 deletion(-)
>>  create mode 100644 tools/waycheck/moretest.c
>>  create mode 100644 tools/waycheck/rough_draw.c
>>  create mode 100644 tools/waycheck/rough_draw.h
>>  create mode 100644 tools/waycheck/waycheck.c
>>  create mode 100644 tools/waycheck/waycheck.dox
>>  create mode 100644 tools/wayland_fixtures/inc/wtst_fixtures.h
>>  create mode 100644 tools/wayland_fixtures/src/wtst_fixtures.c
>>  create mode 100644 tools/zunitc/doc/zunitc.dox
>>  create mode 100644 tools/zunitc/inc/zunitc/zunitc.h
>>  create mode 100644 tools/zunitc/inc/zunitc/zunitc_impl.h
>>  create mode 100644 tools/zunitc/src/main.c
>>  create mode 100644 tools/zunitc/src/zuc_base_logger.c
>>  create mode 100644 tools/zunitc/src/zuc_base_logger.h
>>  create mode 100644 tools/zunitc/src/zuc_collector.c
>>  create mode 100644 tools/zunitc/src/zuc_collector.h
>>  create mode 100644 tools/zunitc/src/zuc_context.h
>>  create mode 100644 tools/zunitc/src/zuc_event.h
>>  create mode 100644 tools/zunitc/src/zuc_event_listener.h
>>  create mode 100644 tools/zunitc/src/zuc_types.h
>>  create mode 100644 tools/zunitc/src/zunitc_impl.c
>>  create mode 100644 tools/zunitc/test/fixtures_test.c
>>  create mode 100644 tools/zunitc/test/zunitc_test.c
>>
> 
>> diff --git a/tools/waycheck/moretest.c b/tools/waycheck/moretest.c
>> new file mode 100644
>> index 0000000..f0c2a4c
>> --- /dev/null
>> +++ b/tools/waycheck/moretest.c
>> @@ -0,0 +1,89 @@
>> +/*
>> + * Copyright © 2015 Samsung Electronics Co., Ltd
>> + *
>> + * Permission to use, copy, modify, distribute, and sell this software and
>> + * its documentation for any purpose is hereby granted without fee, provided
>> + * that the above copyright notice appear in all copies and that both that
>> + * copyright notice and this permission notice appear in supporting
>> + * documentation, and that the name of the copyright holders not be used in
>> + * advertising or publicity pertaining to distribution of the software
>> + * without specific, written prior permission.  The copyright holders make
>> + * no representations about the suitability of this software for any
>> + * purpose.  It is provided "as is" without express or implied warranty.
>> + *
>> + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS
>> + * SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND
>> + * FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY
>> + * SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER
>> + * RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF
>> + * CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
>> + * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> 
> Please, update the license to the current form.
> 
> The pristine copy of the current form is in the COPYING file. Copying
> the license from any existing .c file should be ok now, but if there is
> doubt, cross-check with COPYING.
> 
> Ah, seems like v5 series should have this fixed, I took a peek of the
> diff between the two.
> 

Good. I'd thought that got covered.

>> + */
>> +
>> +#include "config.h"
>> +
>> +#include "zunitc/zunitc.h"
>> +
>> +/*
>> + * The SKIP and FAIL sets of tests are those that will cause 'make check'
>> + * to fail so are disabled by default. They can be re-enabled when working
>> + * on the test framework itself.
>> + */
>> +
>> +/* #define ENABLE_FAIL_TESTS */
> 
>> +#ifdef ENABLE_FAIL_TESTS
>> +ZUC_TEST(more, fail_Versions)
>> +{
>> +	ZUC_EXPECT_FALSE(zuc_has_failure());
>> +	ZUC_EXPECT_FALSE(zuc_has_fatal_failure());
>> +	ZUC_EXPECT_FALSE(zuc_has_nonfatal_failure());
>> +
>> +	ZUC_EXPECT_TRUE(0);
>> +	ZUC_EXPECT_TRUE(!-1);
>> +	ZUC_EXPECT_TRUE(!1);
>> +
>> +	ZUC_EXPECT_FALSE(1);
>> +	ZUC_EXPECT_FALSE(-1);
>> +	ZUC_EXPECT_FALSE(!0);
>> +
>> +	ZUC_EXPECT_EQ(4 + 0, 2 + 3);
>> +
>> +	ZUC_EXPECT_NE(5 - 1, 2 + 2);
>> +
>> +	ZUC_EXPECT_GE(4 - 0, 2 + 3);
>> +
>> +	ZUC_EXPECT_GT(4 - 9, 2 + 3);
>> +
>> +	ZUC_EXPECT_LE(4 + 3, 2 + 3);
>> +
>> +	ZUC_EXPECT_LT(4 + 9, 2 + 3);
>> +
>> +	ZUC_EXPECT_TRUE(zuc_has_failure());
>> +	ZUC_EXPECT_FALSE(zuc_has_fatal_failure());
>> +	ZUC_EXPECT_TRUE(zuc_has_nonfatal_failure());
>> +}
>> +#endif
> 
> 
>> diff --git a/tools/waycheck/rough_draw.h b/tools/waycheck/rough_draw.h
>> new file mode 100644
>> index 0000000..868885f
>> --- /dev/null
>> +++ b/tools/waycheck/rough_draw.h
> 
>> +
>> +/**
>> + * Possible memory formats.
>> + * Very short list for now
>> + */
>> +enum pixel_form {
>> +	PXL_NA = 0,
>> +	PXL_ARGB32,
>> +	PXL_RGB565
>> +};
> 
> Would it be at all possible to *not* invent yet one more pixel format
> enum? Couldn't we use e.g. Pixman formats here? Or wl_shm formats?
> 

I'll check what I can do. The more limiting factor would be trying to
prevent higher-level constructs from leaking down into lower levels
and/or introducing too many dependencies.

I think the main issue with the wl_shm ones were that they require
Wayland to be built and installed to be able to use them. However...
since this is at the "waycheck" level it probably is not a blocker. *If*
it were needed to use these "rough" drawing functions at a lower level
then wl_shm would need to be reconsidered. If not... then no problem.


>> diff --git a/tools/wayland_fixtures/inc/wtst_fixtures.h b/tools/wayland_fixtures/inc/wtst_fixtures.h
>> new file mode 100644
>> index 0000000..3916e3d
>> --- /dev/null
>> +++ b/tools/wayland_fixtures/inc/wtst_fixtures.h
> 
>> +/**
>> + * Creates an instance of the test fixture.
>> + *
>> + * @param display name of the wayland display to connect to or NULL for
>> + * the default.
>> + * @return a pointer to a test fixture upon success, NULL otherwise.
>> + * @see wtst_ctx_destroy()
>> + */
>> +struct wtst_ctx *
>> +wtst_ctx_create(const char *display);
>> +
>> +/**
>> + * Creates an instance of the test fixture with a corresponding surface.
>> + *
>> + * @param display name of the wayland display to connect to or NULL for
>> + * the default.
>> + * @param width the width for the surface.
>> + * @param height the height for the surface.
>> + * @return a pointer to a test fixture upon success, NULL otherwise.
>> + * @see wtst_ctx_destroy()
>> + */
>> +struct wtst_ctx *
>> +wtst_ctx_create_with_surface(const char *display, int width, int height);
>> +
>> +/**
>> + * Creates an instance of the test fixture that is chained.
>> + * Additional registry advertisements will be passed on.
>> + *
>> + * @param display name of the wayland display to connect to or NULL for
>> + * the default.
>> + * @param reg_listener the registry listener to chain in.
>> + * @param data pointer to pass back to registry listener.
>> + * @return a pointer to a test fixture upon success, NULL otherwise.
>> + * @see wtst_ctx_destroy()
>> + */
>> +struct wtst_ctx *
>> +wtst_ctx_create_chained(const char *display,
>> +			struct wl_registry_listener *reg_listener,
>> +			void *reg_data);
>> +
>> +/**
>> + * Creates an instance of the test fixture that is chained and with a
>> + * corresponding surface.
>> + * Additional registry advertisements will be passed on.
>> + *
>> + * @param width the width for the surface.
>> + * @param height the height for the surface.
>> + * @param reg_listener the registry listener to chain in.
>> + * @param data pointer to pass back to registry listener.
>> + * @return a pointer to a test fixture upon success, NULL otherwise.
>> + * @see wtst_ctx_destroy()
>> + */
>> +struct wtst_ctx *
>> +wtst_ctx_create_with_surface_chained(const char *display,
>> +				     int width, int height,
>> +				     struct wl_registry_listener *reg_listener,
>> +				     void *reg_data);
> 
> What does "chained" mean? Where does the word come from?
> 
> Is it just for tests that want to touch a specific global the fixture
> does not handle?

In general, yes. It also allows fixtures to be layered up so that you
can have a Weston fixture that adds onto a generic Wayland fixture.
Things like that.

Definitely sounds like I need to beef up that comment and make intent
clearer.


>> diff --git a/tools/wayland_fixtures/src/wtst_fixtures.c b/tools/wayland_fixtures/src/wtst_fixtures.c
>> new file mode 100644
>> index 0000000..15d100d
>> --- /dev/null
>> +++ b/tools/wayland_fixtures/src/wtst_fixtures.c
> 
>> +#include "config.h"
>> +
>> +#include "wtst_fixtures.h"
>> +
>> +#include <stdint.h>
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <string.h>
>> +#include <sys/mman.h>
>> +#include <unistd.h>
>> +
>> +#include "shared/os-compatibility.h"
>> +#include "shared/zalloc.h"
>> +#include "zunitc/zunitc.h"
>> +
>> +#define ARRAY_LENGTH(a) (sizeof (a) / sizeof (a)[0])
>> +
>> +struct wtst_ctx_private {
>> +
>> +	/** List of all known global advertised by the registry. */
>> +	struct wl_list global_list;
>> +
>> +	/** List of formats supported by the wl_shm. */
>> +	struct wl_list format_list;
>> +
>> +	/** Chained registry listener to invoke. */
>> +	struct wl_registry_listener *chained;
>> +
>> +	/** Data to pass back to chained registry listener. */
>> +	void *chained_data;
>> +
>> +	/** Current main compositor name advertised by the registry. */
>> +	uint32_t compositor_name;
>> +
>> +	/** Current shared memory object name advertised by the registry. */
>> +	uint32_t shm_name;
>> +
>> +	/** Current shell name advertised by the registry. */
>> +	uint32_t shell_name;
>> +};
>> +
>> +/**
>> + * Find the test-seat and set it in client while destroying other inputs.
>> + * @todo probably need to defer input selection to higher layers and not
>> + * hardcode to "test-seat" here.
>> + */
>> +static void
>> +wtst_ctx_set_input(struct wtst_ctx *ctx);
>> +
>> +static void
>> +input_update_devices(struct wtst_input *input);
>> +
>> +static enum wtst_dbgevents_pointer g_pointer_dbg_mask = WTST_DBG_POINTER_ALL;
>> +
>> +void
>> +wtst_set_dump_pointer_events(enum wtst_dbgevents_pointer mask)
>> +{
>> +	g_pointer_dbg_mask = mask;
>> +}
>> +
>> +static void
>> +input_destroy(struct wtst_input *inp)
>> +{
>> +	wl_list_remove(&inp->link);
>> +	wl_seat_destroy(inp->wl_seat);
>> +	free(inp);
>> +}
>> +
>> +struct
>> +wtst_pointer_private
>> +{
>> +	void *data;
>> +	const struct wl_pointer_listener *listener;
>> +	struct wl_list link;
>> +};
> 
>> +static bool is_singleton_interface(const char *interface)
>> +{
>> +	 size_t i = 0;
>> +	const char *known_singletons[] = {
>> +		"wl_compositor",
>> +		"wl_display",
>> +		"wl_compositor",
>> +		"wl_shm",
>> +		"wl_data_device_manager",
>> +	};
>> +
>> +	for (i = 0; i < ARRAY_LENGTH(known_singletons); ++i)
>> +		if (strcmp(known_singletons[i], interface) == 0)
>> +			return true;
>> +
>> +	return false;
>> +}
>> +
>> +static void
>> +add_global_advert(struct wtst_ctx *ctx,
>> +		  uint32_t name,
>> +		  const char *interface,
>> +		  uint32_t version)
>> +{
>> +	if (!ctx || !ctx->private)
>> +		return;
>> +
>> +	/* See if we've got this interface already */
>> +	struct wtst_global *curr = NULL;
>> +	struct wtst_global *tmp = NULL;
>> +	wl_list_for_each(tmp, &ctx->private->global_list, link)
>> +		if (strcmp(tmp->interface, interface) == 0) {
>> +			curr = tmp;
>> +			break;
>> +		}
>> +
>> +	if (curr) {
>> +		/* These are not supposed to change. */
>> +		if (is_singleton_interface(interface)) {
>> +			ZUC_EXPECT_EQ(curr->name, name);
>> +			ZUC_EXPECT_EQ(curr->version, version);
> 
> Actually, if you get here at all, it's already a failure. A singleton
> should not be advertised more than once, even if all properties match.
> 

Yes. Those changing are definitely failure cases, but I'll be sure to
add an explicit one for getting here at all..


>> +		}
>> +	} else {
>> +		struct wtst_global *glbl = zalloc(sizeof(*glbl));
>> +		ZUC_ASSERT_NOT_NULL(glbl);
>> +		glbl->name = name;
>> +		glbl->interface = strdup(interface);
>> +		glbl->version = version;
>> +		wl_list_insert(&ctx->private->global_list, &glbl->link);
>> +	}
>> +}
>> +
> 
>> +struct wtst_shell_surface *
>> +wtst_create_shell_surface(struct wl_compositor *compositor,
>> +			  struct wl_shell *shell)
>> +{
>> +	struct wtst_shell_surface *wss =
>> +		zalloc(sizeof(struct wtst_shell_surface));
> 
> sizeof(*wss) would work too, is shorter, and cannot get the type wrong.

I have no idea how that snuck in, other than refactored copy-n-paste
code. I definitely agree with you on this, and have seen it save code
from bugs many a times.


>> +	ZUC_EXPECT_NOT_NULL(wss);
>> +	if (wss) {
>> +		struct wl_surface *wl_surface =
>> +			wl_compositor_create_surface(compositor);
>> +		ZUC_EXPECT_NOT_NULL(wl_surface);
>> +		wss->surface = wrap_wl_surface(wl_surface);
>> +	}
>> +
>> +	if (wss && wss->surface) {
>> +		wss->wl_shell_surface =
>> +			wl_shell_get_shell_surface(shell,
>> +						   wss->surface->wl_surface);
>> +		ZUC_EXPECT_NOT_NULL(wss->wl_shell_surface);
>> +		if (wss->wl_shell_surface) {
>> +			wl_shell_surface_add_listener(wss->wl_shell_surface,
>> +						      &shell_surface_listener,
>> +						      wss);
>> +			wl_shell_surface_set_toplevel(wss->wl_shell_surface);
>> +			wl_shell_surface_set_user_data(wss->wl_shell_surface,
>> +						       wss);
>> +		}
>> +	}
>> +
>> +	if (wss && (!wss->surface || !wss->wl_shell_surface)) {
>> +		if (wss->surface) {
>> +			struct wl_surface *wl_surface =
>> +				wss->surface->wl_surface;
>> +			free(wss->surface);
>> +			wl_surface_destroy(wl_surface);
>> +		}
>> +		free(wss);
>> +		wss = NULL;
>> +	}
>> +
>> +	return wss;
>> +}
>> diff --git a/tools/zunitc/doc/zunitc.dox b/tools/zunitc/doc/zunitc.dox
>> new file mode 100644
>> index 0000000..4595d7d
>> --- /dev/null
>> +++ b/tools/zunitc/doc/zunitc.dox
> 
>> +/**
>> + at page zunitc zunitc
>> +
>> +- @ref zunitc_overview
>> +- @ref zunitc_execution
>> +  - @ref zunitc_execution_commandline
>> +  - @ref zunitc_execution_matching
>> +  - @ref zunitc_execution_wildcards
>> +  - @ref zunitc_execution_repeat
>> +  - @ref zunitc_execution_randomize
>> +- @ref zunitc_fixtures
>> +- @ref zunitc_functions
>> +
>> + at section zunitc_overview Overview
>> +
>> +A simple test framework in plain C suitable for basic unit and integration testing.
>> +
>> +The main rationale for creating this framework was to have a simple to use testing
>> +framework with tests implemented in C using common patterns and under a
>> +compatible license. The structure of the test code and macro use is intended to
>> +follow common patterns established by frameworks such as Boost Test and Google Test.
>> +
>> +
>> +To get started, one or more tests should be defined via ZUC_TEST() and/or
>> +ZUC_TEST_F(), which set up automatic test registration via gcc extensions.
>> +To actually execute tests, ZUC_RUN_TESTS() should be called.
>> +
>> +
>> +Tests can use several ZUC_EXPECT_* or ZUC_ASSERT_* checks to validate
>> +conditions. The ZUC_EXPECT_* ones upon failure will mark the current test
>> +as failing, but allow execution to continue. On the other hand, the
>> +ZUC_ASSERT_* tests will mark the current test as failed and then immediately
>> +terminate the test.
>> +
>> +The set of non-fatal test checks are
>> +
>> +- ZUC_EXPECT_TRUE()
>> +- ZUC_EXPECT_FALSE()
>> +- ZUC_EXPECT_EQ()
>> +- ZUC_EXPECT_NE()
>> +- ZUC_EXPECT_LT()
>> +- ZUC_EXPECT_LE()
>> +- ZUC_EXPECT_GT()
>> +- ZUC_EXPECT_GE()
>> +
>> +The set of fatal test checks are
>> +
>> +- ZUC_ASSERT_TRUE()
>> +- ZUC_ASSERT_FALSE()
>> +- ZUC_ASSERT_EQ()
>> +- ZUC_ASSERT_NE()
>> +- ZUC_ASSERT_LT()
>> +- ZUC_ASSERT_LE()
>> +- ZUC_ASSERT_GT()
>> +- ZUC_ASSERT_GE()
> 
> Missing the string macros. Would be much nicer to autogenerate these
> lists.

I'll have to look into some build magic to do this well. Generating the
temp files might take extra time, especially to be sure distcheck stays
bulletproof.

>> +
>> +Unconditional test values for logging and termination are
>> +- ZUC_SKIP()
>> +- ZUC_FAIL()
>> +- ZUC_FATAL()
>> +
>> +Unconditional message logging for failure cases only is
>> +- ZUC_TRACEPOINT()
>> +
>> + at section zunitc_execution Controlling Execution
>> +
>> +To control execution, the various zuc_set_* functions can be called before invoking ZUC_RUN_TESTS(). 
>> +
>> + at subsection zunitc_execution_commandline Commandline Parameters
>> +
>> +The current implementation defers processing of command-line parameters to the main application hosting the testing. It is possible that a helper to process certain parameters may be added.
>> +
>> + at subsection zunitc_execution_matching Matching Patterns for Tests
>> +
>> +The function zuc_set_filter() can be used to specify a pattern for matching or excluding tests from a run. The general form is
>> + match1[:match2[:match3..n]][:-exclude1[:exclude2[:exclude3..n]]]
>> +
>> + at subsection zunitc_execution_wildcards Matching Wildcards
>> +
>> +Wildcards can be used in the match/exclude patterns and recognize the following two special characters:
>> +- '*' matches any number of characters including zero.
>> +- '?' matches any single character.
>> +
>> +Calling zuc_list_tests() after zuc_set_filter() can be done to show the effects of the matching without needing to actually run tests.
>> +
>> + at subsection zunitc_execution_repeat Repeating Tests
>> +
>> +Setting the repeat count higher than 1 ( via zuc_set_repeat() ) will cause the tests to be executed several times in a row. This can be useful for stress testing, checking for leaks, etc.
>> +
>> + at subsection zunitc_execution_randomize Randomizing Tests
>> +
>> +Test ordering can be randomized by setting a non-zero positive value to zuc_set_random(). Setting it to 1 will cause the framework to pick a random seed based on the time. A value greater than 1 will be taken as a random seed itself. And setting it to 0 will disable randomization and allow the test to be executed in their natural ordering.
>> +
>> + at section zunitc_fixtures Fixtures
>> +
>> +Per-suite and per-test setup and teardown fixtures can be implemented by defining an instance of struct zuc_fixture and using it as the first parameter to ZUC_TEST_F().
>> +
>> + at section zunitc_functions Functions
>> +
>> +- zuc_get_fixture_data()
>> +- ZUC_TEST()
>> +- ZUC_TEST_F()
>> +- ZUC_RUN_TESTS()
>> +- zuc_cleanup()
>> +- zuc_list_tests()
>> +- zuc_set_filter()
>> +- zuc_set_random()
>> +- zuc_set_spawn()
>> +- zuc_set_output_tap()
>> +- zuc_set_output_junit()
>> +- zuc_has_skip()
>> +- zuc_has_failure()
>> +- zuc_has_fatal_failure()
>> +- zuc_has_nonfatal_failure()
>> +
>> +*/
>> diff --git a/tools/zunitc/inc/zunitc/zunitc.h b/tools/zunitc/inc/zunitc/zunitc.h
>> new file mode 100644
>> index 0000000..ff01323
>> --- /dev/null
>> +++ b/tools/zunitc/inc/zunitc/zunitc.h
>> @@ -0,0 +1,681 @@
> 
>> +#ifndef Z_UNIT_C_H
>> +#define Z_UNIT_C_H
>> +
>> +#include <limits.h>
>> +#include <stdarg.h>
>> +#include <stdbool.h>
>> +#include <stdint.h>
>> +
>> +#include "zunitc/zunitc_impl.h"
>> +
>> +#if !__GNUC__
>> +#error Framework currently requires gcc or compatible compiler.
>> +#endif
>> +
>> +#if INTPTR_MAX < INT_MAX
>> +#error Odd platform requires rework of value type from intptr_t to custom.
>> +#endif
>> +
>> +/**
>> + * @file
>> + * Simple unit test framework declarations.
>> + */
>> +
>> +#ifdef __cplusplus
>> +extern "C" {
>> +#endif
>> +
>> +/**
>> + * @page zunitc
>> + */
>> +
>> +/**
>> + * Structure to use when defining a test fixture.
>> + * @note likely pending refactoring as use cases are refined.
>> + */
>> +struct zuc_fixture {
>> +	/**
>> +	 * Initial optional seed data to pass to setup functions and/or tests.
>> +	 */
>> +	const void *data;
>> +
>> +	/**
>> +	 * Per-suite setup called before invoking any of the tests
>> +	 * contained in the suite.
>> +	 *
>> +	 * @return a pointer to test data, or NULL.
>> +	 */
>> +	void *(*set_up_test_case)(const void *data);
>> +
>> +	/**
>> +	 * Per-suite tear-down called after invoking all of the tests
>> +	 * contained in the suite.
>> +	 *
>> +	 * @param data pointer returned from the setup function.
>> +	 */
>> +	void (*tear_down_test_case)(void *data);
>> +
>> +	/**
>> +	 * Setup called before running each of the tests in the suite.
>> +	 *
>> +	 * @param data optional data from suite setup, or NULL.
>> +	 * @return a pointer to test data, or NULL.
>> +	 */
>> +	void *(*set_up)(void *data);
>> +
>> +	/**
>> +	 * Tear-down called after running each of the tests in the suite.
>> +	 *
>> +	 * @param data pointer returned from the setup function.
>> +	 */
>> +	void (*tear_down)(void *data);
>> +};
> 
>> +/**
>> + * Initializes the test framework and consumes any known command-line
>> + * parameters from the list.
>> + * The exception is 'h/help' which will be left in place for follow-up
>> + * processing by the hosting app if so desired.
>> + *
>> + * @param argc pointer to argc value to read and possibly change.
>> + * @param argv array of parameter pointers to read and possibly change.
>> + * @param help_flagged if non-NULL will be set to true if the user
>> + * specifies the help flag (and framework help has been output).
>> + * @return EXIT_SUCCESS upon success setting or help, EXIT_FAILURE otherwise.
>> + */
>> +int zuc_initialize(int *argc, char *argv[], bool *help_flagged);
>> +
>> +/**
>> + * Runs all tests that have been registered.
>> + * Expected return values include EXIT_FAILURE if any errors or failures
>> + * have occurred, ::ZUC_EXIT_SKIP if no failures have occurred but at least
>> + * one test reported skipped, otherwise EXIT_SUCCESS if nothing of note
>> + * was recorded.
>> + *
>> + * @return expected exit status - normally EXIT_SUCCESS, ::ZUC_EXIT_SKIP,
>> + * or EXIT_FAILURE.
>> + */
>> +#define ZUC_RUN_TESTS() \
>> +	zucimpl_run_tests()
> 
> Why is this a macro when it just calls a function?
> I'm also slightly confused what the return value is. Does "expected"
> mean that I should do exit() with the returned value?

Initially this was to be consistent with other frameworks. I'll add a
note about passing the value to exit().


>> +
>> +/**
>> + * Clears the test system in preparation for application shutdown.
>> + */
>> +void
>> +zuc_cleanup(void);
>> +
>> +/**
>> + * Displays all known tests.
>> + * The list returned is affected to any filtering in place.
>> + *
>> + * @see zuc_set_filter()
>> + */
>> +void
>> +zuc_list_tests(void);
>> +
>> +/**
>> + * Sets the filter string to use for tests.
>> + * The format is a series of patterns separated by a colon, with wildcards
>> + * and an optional flag for negative matching. For wildcards, the '*'
>> + * character will match any sequence and the '?' character will match any
>> + * single character.
>> + * The '-' character at the start of a pattern marks the end of the
>> + * patterns required to match and the begining of patterns that names
>> + * must not match.
>> + *
>> + * @param filter the filter string to apply to tests.
>> + */
>> +void
>> +zuc_set_filter(const char *filter);
>> +
>> +/**
>> + * Trigger specific failure/signal upon test failures; useful when
>> + * running under a debugger.
>> + *
>> + * @param break_on_failure true to cause a break when tests fail, false to
>> + * allow normal operation upon failures.
>> + */
>> +void
>> +zuc_set_break_on_failure(bool break_on_failure);
> 
> What does the break do exactly?

It probably does not do as much as it should currently. However it does
cause gdb to catch a stop in execution. Some frameworks take it a step
further and actually invoke the proper debugger.

>> +
>> +/**
>> + * Sets the number of times to repeat the tests.
>> + * Any number higher than 1 will cause the tests to be repeated the
>> + * specified number of times.
>> + *
>> + * @param repeat number of times to repeat the tests.
>> + */
>> +void
>> +zuc_set_repeat(int repeat);
>> +
>> +/**
>> + * Randomizes the order in which tests are executed.
>> + * A value of 0 (the default) means tests are executed in their natural
>> + * ordering. A value of 1 will pick a random seed based on the time to
>> + * use for running tests in a pseudo-random order. A value greater than 1
>> + * will be used directly for the initial seed.
>> + *
>> + * If the tests are also repeated, the seed will be incremented for each
>> + * subsequent run.
>> + *
>> + * @param random 0|1|seed value.
>> + * @see zuc_set_repeat()
>> + */
>> +void
>> +zuc_set_random(int random);
>> +
>> +/**
>> + * Controls whether or not to run the tests as forked child processes.
>> + *
>> + * @param spawn true to spawn each test in a forked child process,
>> + * false to run tests directly.
>> + */
>> +void
>> +zuc_set_spawn(bool spawn);
> 
> There are lots of zuc_set_*() functions. What are the default settings?

Should it just document the defaults? (which, of course, will make me
commit to their behavior). And/or would these need gettors?


>> +
>> +/**
>> + * Defines a test case that can be registered to run.
>> + */
>> +#define ZUC_TEST(tcase, test) \
>> +	static void zuctest_##tcase##_##test(void); \
>> +	\
>> +	const struct zuc_registration zzz_##tcase##_##test \
>> +	__attribute__ ((section ("zuc_tsect"))) = \
>> +	{ \
>> +		#tcase, #test, 0,		\
>> +		zuctest_##tcase##_##test,	\
>> +		0				\
>> +	}; \
>> +	\
>> +	static void zuctest_##tcase##_##test(void)
>> +
>> +/**
>> + * Defines a test case that can be registered to run along with setup/teardown
>> + * support per-test and/or per test case.
>> + *
>> + * @note likely pending refactoring as use cases are refined.
>> + */
>> +#define ZUC_TEST_F(tcase, test) \
>> +	static void zuctest_##tcase##_##test(void *data); \
>> +	\
>> +	const struct zuc_registration zzz_##tcase##_##test \
>> +	__attribute__ ((section ("zuc_tsect"))) = \
>> +	{ \
>> +		#tcase, #test, &tcase,		\
>> +		0,				\
>> +		zuctest_##tcase##_##test	\
>> +	}; \
>> +	\
>> +	static void zuctest_##tcase##_##test(void *data)
>> +
>> +
> 
>> +
>> +/**
>> + * Terminates the current test without marking it as failed.
>> + *
>> + * @param message the message to log as to why the test has been skipped.
>> + */
>> +#define ZUC_SKIP(message) \
>> +	do { \
>> +		zucimpl_terminate(__FILE__, __LINE__, false, false, #message); \
>> +		return; \
>> +	} \
>> +	while (0)
> 
> Hmm, that "return". That needs extra documentation so that people don't
> call it from a sub-function without checking for skips in the caller.
> It also makes the assumption of the return type being void.

Yes. This is where usage starts to get a little tricky.

Translating a common idiom from gtest we could have

    some_function_that_checks();
    ZUC_ASSERT(!zuc_has_failure());

I've also been pondering adding a parallel set that is non-void, similar
to GTK+/GLib's g_return_val_if_fail()


>> +
>> +/**
>> + * Marks the current test as failed, but continues execution.
>> + *
>> + * @param message the message to log as to why the test has failed.
>> + */
>> +#define ZUC_FAIL(message) \
>> +	zucimpl_terminate(__FILE__, __LINE__, true, false, #message)
>> +
>> +/**
>> + * Terminates the current test and marks it as failed.
>> + *
>> + * @param message the message to log as to why the test has failed.
>> + */
>> +#define ZUC_FATAL(message) \
>> +	do { \
>> +		zucimpl_terminate(__FILE__, __LINE__, true, true, #message); \
>> +		return; \
>> +	} \
>> +	while (0)
> 
> The same comments about "return" here.

Yes, same issue.


>> +
>> +/**
>> + * Marks the current test as failed with a fatal issue, but does not
>> + * immediately return from the current function. ZUC_FATAL() is normally
>> + * preferred, but when further cleanup is needed, or the current function
>> + * needs to return a value, this macro may be required.
>> + *
>> + * @param message the message to log as to why the test has failed.
>> + * @see ZUC_FATAL()
>> + */
>> +#define ZUC_MARK_FATAL(message) \
>> +	do { \
>> +		zucimpl_terminate(__FILE__, __LINE__, true, true, #message); \
>> +	} \
>> +	while (0)
>> +
>> +/**
>> + * Creates a message that will be processed in the case of failure.
>> + * If the test encounters any failures (fatal or non-fatal) then these
>> + * messages are included in output. Otherwise they are discarded at the
>> + * end of the test run.
>> + *
>> + * @param message the format string style message.
>> + */
>> +#define ZUC_TRACEPOINT(message, ...) \
>> +	zucimpl_tracepoint(__FILE__, __LINE__, message, ##__VA_ARGS__);
>> +
>> +/**
>> + * Verifies that the specified expression is true and marks the test as failed
>> + * if it is not.
>> + *
>> + * @param condition the expression that is expected to be true.
>> + * @note it is far better to use a more specific check when possible
>> + * (e.g. ZUC_EXPECT_EQ(), ZUC_EXPECT_NE(), etc.)
>> + */
>> +#define ZUC_EXPECT_TRUE(condition) \
>> +	zucimpl_expect_pred2(__FILE__, __LINE__,		\
>> +			     ZUC_OP_TRUE, ZUC_VAL_INT, false,	\
>> +			     (condition), 0, #condition, "")
>> +
> 
>> +/**
>> + * Internal use macro for ASSERT implementation.
>> + * Should not be used directly in code.
>> + */
>> +#define ZUCIMPL_ASSERT(opcode, valtype, lhs, rhs) \
>> +	do { \
>> +		if (zucimpl_expect_pred2(__FILE__, __LINE__, \
>> +					 (opcode), (valtype), true, \
>> +					 (intptr_t)(lhs), (intptr_t)(rhs), \
>> +					 #lhs, #rhs)) { \
>> +			return; \
>> +		} \
>> +	} \
>> +	while (0)
> 
> Again "return". Looks like all ZUC_ASSERT_* macros cause a "return".
> Are we forbidden to use sub-functions in our test code?

Well... not forbidden, just needs to verify after calling them.

>> +
>> +
>> +/**
>> + * Verifies that the specified expression is true, marks the test as failed
>> + * and terminates the test if it is not.
>> + *
>> + * @param condition the expression that is expected to be true.
>> + * @note it is far better to use a more specific check when possible
>> + * (e.g. ZUC_EXPECT_EQ(), ZUC_EXPECT_NE(), etc.)
>> + */
>> +#define ZUC_ASSERT_TRUE(condition) \
>> +	ZUCIMPL_ASSERT(ZUC_OP_TRUE, ZUC_VAL_INT, condition, 0)
> 
>> diff --git a/tools/zunitc/inc/zunitc/zunitc_impl.h b/tools/zunitc/inc/zunitc/zunitc_impl.h
>> new file mode 100644
>> index 0000000..eb3f861
>> --- /dev/null
>> +++ b/tools/zunitc/inc/zunitc/zunitc_impl.h
> 
>> +#ifndef Z_UNIT_C_IMPL_H
>> +#define Z_UNIT_C_IMPL_H
>> +
>> +/**
>> + * @file
>> + * Internal details to bridge the public API - should not be used
>> + * directly in user code.
>> + */
>> +
>> +#include <stdbool.h>
>> +#include <stdint.h>
>> +
>> +#ifdef __cplusplus
>> +extern "C" {
>> +#endif
>> +
>> +enum zuc_check_op
>> +{
>> +	ZUC_OP_TRUE,
>> +	ZUC_OP_FALSE,
>> +	ZUC_OP_NULL,
>> +	ZUC_OP_NOT_NULL,
>> +	ZUC_OP_EQ,
>> +	ZUC_OP_NE,
>> +	ZUC_OP_GE,
>> +	ZUC_OP_GT,
>> +	ZUC_OP_LE,
>> +	ZUC_OP_LT,
>> +	ZUC_OP_TERMINATE,
>> +	ZUC_OP_TRACEPOINT
>> +};
>> +
>> +enum zuc_check_valtype
>> +{
>> +	ZUC_VAL_INT,
>> +	ZUC_VAL_CSTR,
>> +	ZUC_VAL_PTR,
>> +};
>> +
>> +typedef void (*zucimpl_test_fn)(void);
>> +
>> +typedef void (*zucimpl_test_fn_f)(void *);
>> +
>> +/**
>> + * Internal use structure for automatic test case registration.
>> + * Should not be used directly in code.
>> + */
>> +struct zuc_registration {
>> +	const char *tcase;		/**< Name of the test case. */
>> +	const char *test;		/**< Name of the specific test. */
>> +	const struct zuc_fixture* fxt;	/**< Optional fixture for test/case. */
>> +	zucimpl_test_fn fn;		/**< function implementing base test. */
>> +	zucimpl_test_fn_f fn_f;	/**< function implementing test with
>> +					   fixture. */
>> +} __attribute__ ((aligned (32)));
>> +
>> +
>> +int
>> +zucimpl_run_tests(void);
>> +
>> +void
>> +zucimpl_terminate(char const *file, int line,
>> +		  bool fail, bool fatal, const char *msg);
>> +
>> +int
>> +zucimpl_tracepoint(char const *file, int line, const char *fmt, ...)
>> +	__attribute__ ((format (printf, 3, 4)));
>> +
>> +int
>> +zucimpl_expect_pred2(char const *file, int line,
>> +		     enum zuc_check_op, enum zuc_check_valtype valtype,
>> +		     bool fatal,
>> +		     intptr_t lhs, intptr_t rhs,
>> +		     const char *lhs_str, const char* rhs_str);
>> +
>> +#ifdef __cplusplus
>> +}
>> +#endif
>> +
>> +#endif /* Z_UNIT_C_IMPL_H */
>> diff --git a/tools/zunitc/src/main.c b/tools/zunitc/src/main.c
>> new file mode 100644
>> index 0000000..3e155d8
>> --- /dev/null
>> +++ b/tools/zunitc/src/main.c
> 
>> +#include "config.h"
>> +
>> +/*
>> + * Common main() for test programs.
>> + */
>> +
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +
>> +#include "zunitc/zunitc.h"
>> +
>> +int
>> +main(int argc, char* argv[])
>> +{
>> +	bool helped = false;
>> +	int rc = zuc_initialize(&argc, argv, &helped);
>> +
>> +	if ((rc == EXIT_SUCCESS) && !helped) {
>> +		rc = ZUC_RUN_TESTS();
>> +	}
>> +
>> +	zuc_cleanup();
>> +	return rc;
>> +}
> 
> Ok, nice.
> 
> 
>> diff --git a/tools/zunitc/src/zuc_base_logger.c b/tools/zunitc/src/zuc_base_logger.c
>> new file mode 100644
>> index 0000000..3985f90
>> --- /dev/null
>> +++ b/tools/zunitc/src/zuc_base_logger.c
> 
> Mmm, pluggable stuff.
> 
> 
>> diff --git a/tools/zunitc/src/zuc_collector.c b/tools/zunitc/src/zuc_collector.c
>> new file mode 100644
>> index 0000000..d3255cd
>> --- /dev/null
>> +++ b/tools/zunitc/src/zuc_collector.c
>> @@ -0,0 +1,424 @@
> 
> Lots of functions in this file could use 'static' in the declaration,
> right? Well, you have declarations with static, definitions without
> static, I find that (and explicit declarations when they are not really
> needed) confusing.

Good. Info for me to add to the coding style guide. Will make it easier
for the next contributors know what are preferences and what things just
happened to be in there.


>> diff --git a/tools/zunitc/src/zuc_collector.h b/tools/zunitc/src/zuc_collector.h
>> new file mode 100644
>> index 0000000..1febf53
>> --- /dev/null
>> +++ b/tools/zunitc/src/zuc_collector.h
> 
>> +#ifndef ZUC_COLLECTOR_H
>> +#define ZUC_COLLECTOR_H
>> +
>> +struct zuc_event_listener;
>> +struct zuc_test;
>> +
>> +/**
>> + * Creates a new instance of an even collector that will attatch events
>> + * to the current test directly or via connection from child to parent.
>> + *
>> + * @param pipe_fd pointer to the file descriptor to use for communication if
>> + * needed. If the value is -1 the events will be attached directly to the
>> + * current test. Otherwise events will be passed back via IPC over this
>> + * pipe with the expectation that the payload will be handled in the parent
>> + * process via zuc_process_message().
>> + * @return a new collector intance.
>> + * @see zuc_process_message()
>> + */
>> +struct zuc_event_listener *
>> +zuc_collector_create(int *pipe_fd);
>> +
>> +/**
>> + * Reads events from the given pipe and processes them.
>> + *
>> + * @param test the currently active test to attache events for.
>> + * @param pipe_fd the file descriptor of the pipe to read from.
>> + * @return a positive value if a message was received, 0 if the end has
>> + * been reached and -1 if an error has occurred.
>> + */
>> +int
>> +zuc_process_message(struct zuc_test *test, int pipe_fd);
> 
> Ah, message passing between processes. Nice. I suppose if you fork(),
> and you want structured output like those xml stuffs, nothing simpler
> will really cut it?

Correct. I've coded mechanisms, used different libs with them... many
different ways. Definitiely in the simpler-is-better camp here. Getting
things so that the fork() will work, but is still optional, and behaves
in ways not to confuse valgrind was the main target.

I definitely don't want to hook in SOAP here or anything (and, yes, I
have had that pushed by architects before in such scenarios)


>> diff --git a/tools/zunitc/src/zunitc_impl.c b/tools/zunitc/src/zunitc_impl.c
>> new file mode 100644
>> index 0000000..776a30b
>> --- /dev/null
>> +++ b/tools/zunitc/src/zunitc_impl.c
> 
>> +#include "config.h"
>> +
>> +#include <fnmatch.h>
>> +
>> +#include <errno.h>
>> +#include <fcntl.h>
>> +#include <stdarg.h>
>> +#include <stdbool.h>
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <string.h>
>> +#include <sys/types.h>
>> +#include <sys/wait.h>
>> +#include <time.h>
>> +#include <unistd.h>
>> +
>> +#include "zunitc/zunitc_impl.h"
>> +#include "zunitc/zunitc.h"
>> +
>> +#include "zuc_base_logger.h"
>> +#include "zuc_collector.h"
>> +#include "zuc_context.h"
>> +#include "zuc_event_listener.h"
>> +
>> +#include "shared/config-parser.h"
>> +#include "shared/zalloc.h"
>> +
>> +#define ARRAY_LENGTH(a) ((int) (sizeof (a) / sizeof (a)[0]))
>> +
>> +/*
>> + * If CLOCK_MONOTONIC is present on the system it will give us reliable
>> + * results under certain edge conditions that normally require manual
>> + * admin actions to trigger. If not, CLOCK_REALTIME is a reasonable
>> + * fallback.
>> + */
>> +#if _POSIX_MONOTONIC_CLOCK
>> +static const clockid_t TARGET_TIMER = CLOCK_MONOTONIC;
>> +#else
>> +static const clockid_t TARGET_TIMER = CLOCK_REALTIME;
>> +#endif
> 
> I do wonder if build time checks are appropriate. I did a runtime check
> for weston's presentation clock. Not that it would matter here.

>From some general experience here it seems POSIX makes for a fair
compile time check. Especially if one favors compile-time safety over
run-time.

This might matter for OSX IIRC.


>> +static char const DISABLED_PREFIX[] = "DISABLED_";
>> +
>> +#define MS_PER_SEC 1000L
>> +#define NANO_PER_MS 1000000L
>> +
>> +/**
>> + * Simple single-linked list structure.
>> + */
>> +struct zuc_slinked {
>> +	void *data;
>> +	struct zuc_slinked *next;
>> +};
>> +
>> +static struct zuc_context g_ctx = {
>> +	.case_count = 0,
>> +	.cases = NULL,
>> +
>> +	.fatal = false,
>> +	.repeat = 0,
>> +	.random = 0,
>> +	.spawn = true,
>> +	.break_on_failure = false,
>> +	.fds = {-1, -1},
>> +
>> +	.listeners = NULL,
>> +
>> +	.curr_case = NULL,
>> +	.curr_test = NULL,
>> +};
>> +
>> +static char *g_progname = NULL;
>> +static char *g_progbasename = NULL;
>> +
>> +static void
>> +initialize(void);
>> +
>> +static void
>> +register_tests(void);
>> +
>> +static void
>> +free_test_case(struct zuc_case *test_case);
>> +
>> +static struct zuc_test *
>> +create_test(int order, zucimpl_test_fn fn, zucimpl_test_fn_f fn_f,
>> +	    char const *case_name, char const *test_name,
>> +	    struct zuc_case *parent);
>> +
>> +static void
>> +free_test(struct zuc_test *test);
>> +
>> +static void
>> +free_events(struct zuc_event **events);
>> +
>> +static void
>> +migrate_deferred_events(struct zuc_test *test, bool transferred);
>> +
>> +static int
>> +compare_regs(const void *lhs, const void *rhs);
>> +
>> +static int
>> +compare_case_order(const void *lhs, const void *rhs);
>> +
>> +static int
>> +compare_test_order(const void *lhs, const void *rhs);
>> +
>> +static void
>> +order_cases(int count, struct zuc_case **cases);
>> +
>> +static void
>> +shuffle_cases(int count, struct zuc_case **cases, unsigned int seed);
>> +
>> +static void
>> +filter_cases(int *count, struct zuc_case **cases, char const *filter);
>> +
>> +static unsigned int
>> +get_seed_from_time(void);
>> +
>> +/**
>> + * A very simple matching that is compatible with the algorithm used in
>> + * Google Test.
>> + *
>> + * @param wildcard sequence of '?', '*' or normal characters to match.
>> + * @param str string to check for matching.
>> + * @return true if the wildcard matches the entire string, false otherwise.
>> + */
>> +static bool
>> +wildcard_matches(char const *wildcard, char const *str);
>> +
>> +static bool
>> +test_has_skip(struct zuc_test *test);
>> +
>> +static bool
>> +test_has_failure(struct zuc_test *test);
>> +
>> +static bool
>> +test_has_fatal_failure(struct zuc_test *test);
>> +
>> +static bool
>> +test_has_nonfatal_failure(struct zuc_test *test);
>> +
>> +static void
>> +reset_test_values(struct zuc_case **cases, int case_count);
>> +
>> +static void
>> +mark_failed(struct zuc_test *test, enum zuc_fail_state state);
>> +
>> +static void
>> +mark_single_failed(struct zuc_test *test, enum zuc_fail_state state);
>> +
>> +static int
>> +run_single_pass(void);
>> +
>> +static void
>> +run_single_case(struct zuc_case *test_case);
>> +
>> +static void
>> +run_single_test(struct zuc_test *test, const struct zuc_fixture *fxt,
>> +		void *case_data, bool spawn);
>> +
>> +static void
>> +spawn_test(struct zuc_test *test, void *test_data,
>> +	   void (*cleanup_fn)(void *data), void *cleanup_data);
>> +
>> +typedef int (*comp_pred2)(intptr_t lhs, intptr_t rhs);
>> +
>> +static comp_pred2
>> +get_pred2(enum zuc_check_op op, enum zuc_check_valtype valtype);
>> +
>> +/* for outputting results */
>> +static void
>> +dispatch_pre_run(struct zuc_context *ctx, int pass_count, int pass_num,
>> +		 int seed, const char *filter);
>> +
>> +static void
>> +dispatch_run_started(struct zuc_context *ctx, int live_case_count,
>> +		     int live_test_count, int disabled_count);
>> +
>> +static void
>> +dispatch_run_ended(struct zuc_context *ctx,
>> +		   int live_case_count, int live_test_count, int total_passed,
>> +		   int total_failed, int total_disabled, long total_elapsed);
>> +
>> +static void
>> +dispatch_case_started(struct zuc_context *ctx, struct zuc_case *test_case,
>> +		      int live_test_count, int disabled_count);
>> +
>> +static void
>> +dispatch_case_ended(struct zuc_context *ctx, struct zuc_case *test_case);
>> +
>> +static void
>> +dispatch_test_started(struct zuc_context *ctx, struct zuc_test *test);
>> +
>> +static void
>> +dispatch_test_ended(struct zuc_context *ctx, struct zuc_test *test);
>> +
>> +static void
>> +dispatch_test_disabled(struct zuc_context *ctx, struct zuc_test *test);
>> +
>> +static void
>> +dispatch_check_triggered(struct zuc_context *ctx, char const *file, int line,
>> +			 enum zuc_fail_state state, enum zuc_check_op op,
>> +			 enum zuc_check_valtype valtype,
>> +			 intptr_t val1, intptr_t val2,
>> +			 const char *expr1, const char *expr2);
>> +
>> +static void
>> +dispatch_collect_event(struct zuc_context *ctx,char const *file, int line,
>> +		       const char *expr1);
> 
> Do you /really/ need all these declarations?

Probably not most. I'll purge my brain of C++ habits and give it another
pass.


>> +int
>> +zuc_initialize(int *argc, char *argv[], bool *help_flagged)
>> +{
>> +	int rc = EXIT_FAILURE;
>> +	int opt_help = 0;
>> +	int opt_nofork = 0;
>> +	int opt_list = 0;
>> +	int opt_repeat = 0;
>> +	int opt_random = 0;
>> +	int opt_break_on_failure = 0;
>> +	char *opt_filter = NULL;
>> +
>> +	char *help_param = NULL;
>> +	int argc_in = *argc;
>> +
>> +	const struct weston_option options[] = {
>> +		{ WESTON_OPTION_BOOLEAN, "zuc-nofork", 0, &opt_nofork },
>> +		{ WESTON_OPTION_BOOLEAN, "zuc-list-tests", 0, &opt_list },
>> +		{ WESTON_OPTION_INTEGER, "zuc-repeat", 0, &opt_repeat },
>> +		{ WESTON_OPTION_INTEGER, "zuc-random", 0, &opt_random },
>> +		{ WESTON_OPTION_BOOLEAN, "zuc-break-on-failure", 0,
>> +		  &opt_break_on_failure },
>> +		{ WESTON_OPTION_STRING, "zuc-filter", 0, &opt_filter },
>> +	};
>> +
>> +	/*
>> +	 *If a test binary is linked to our libzunitcmain it might want
>> +	 * to access the program 'name' from argv[0]
>> +	 */
>> +	free(g_progname);
>> +	g_progname = NULL;
>> +	free(g_progbasename);
>> +	g_progbasename = NULL;
>> +	if ((*argc > 0) && argv) {
>> +		char *path = NULL;
>> +		char *base = NULL;
>> +
>> +		g_progname = strdup(argv[0]);
>> +
>> +		/* basename() might modify the input, so needs a writeable
>> +		 * string.
>> +		 * It also may return a statically allocated buffer subject to
>> +		 * being overwritten so needs to be dup'd.
>> +		 */
>> +		path = strdup(g_progname);
>> +		base = basename(path);
>> +		g_progbasename = strdup(base);
>> +		free(path);
>> +	} else {
>> +		g_progname = strdup("");
>> +		printf("%s:%d: warning: No valid argv[0] for initialization\n",
>> +		       __FILE__, __LINE__);
>> +	}
>> +
>> +
>> +	initialize();
>> +	if (g_ctx.fatal)
>> +		return EXIT_FAILURE;
>> +
>> +	if (help_flagged)
>> +		*help_flagged = false;
>> +
>> +	{
>> +		/* Help param will be a special case and need restoring. */
>> +		int i = 0;
>> +		char **argv_in = NULL;
>> +		const struct weston_option help_options[] = {
>> +			{ WESTON_OPTION_BOOLEAN, "help", 'h', &opt_help },
>> +		};
>> +		argv_in = zalloc(sizeof(char *) * argc_in);
>> +		if (!argv_in) {
>> +			printf("%s:%d: error: alloc failed.\n",
>> +			       __FILE__, __LINE__);
>> +			return EXIT_FAILURE;
>> +		}
>> +		for (i = 0; i < argc_in; ++i)
>> +			argv_in[i] = argv[i];
>> +
>> +		parse_options(help_options, ARRAY_LENGTH(help_options),
>> +			      argc, argv);
>> +		if (*argc < argc_in) {
>> +			for (i = 1; (i < argc_in) && !help_param; ++i) {
>> +				bool found = false;
>> +				int j = 0;
>> +				for (j = 0; (j < *argc) && !found; ++j)
>> +					found = (argv[j] == argv_in[i]);
>> +
>> +				if (!found)
>> +					help_param = argv_in[i];
>> +			}
>> +		}
>> +		free(argv_in);
> 
> I wonder if it wouldn't be simpler to just scan for it manually, rather
> than do all the tricks to be able to call parse_options()?

That was how I first coded it. However there was some concern that it
could be breaking modularity and possibly forwards compatibility. If the
commmand-line parser gets updated any time in the future then manually
scanning for it here could get broken.

Having worked with many different parsers, there are definitely
behaviors that people might request that could affect this.


>> +	}
>> +
>> +	parse_options(options, ARRAY_LENGTH(options), argc, argv);
>> +
>> +	if (help_param && (*argc < argc_in))
>> +		argv[(*argc)++] = help_param;
>> +
>> +	if (opt_filter) {
>> +		zuc_set_filter(opt_filter);
>> +		free(opt_filter);
>> +	}
>> +
>> +	if (opt_help) {
>> +		printf("Usage: %s [OPTIONS]\n"
>> +		       "  --zuc-break-on-failure\n"
>> +		       "  --zuc-filter=FILTER\n"
>> +		       "  --zuc-list-tests\n"
>> +		       "  --zuc-nofork\n"
>> +		       "  --zuc-output-xml\n"
>> +		       "  --zuc-random=N            [0|1|<seed number>]\n"
>> +		       "  --zuc-repeat=N\n"
>> +		       "  --help\n",
>> +		       argv[0]);
>> +		if (help_flagged)
>> +			*help_flagged = true;
>> +		rc = EXIT_SUCCESS;
>> +	} else if (opt_list) {
>> +		zuc_list_tests();
>> +		rc = EXIT_FAILURE;
>> +	} else {
>> +		zuc_set_repeat(opt_repeat);
>> +		zuc_set_random(opt_random);
>> +		zuc_set_spawn(!opt_nofork);
>> +		zuc_set_break_on_failure(opt_break_on_failure);
>> +		rc = EXIT_SUCCESS;
>> +	}
>> +
>> +	return rc;
>> +}
> 
>> +bool
>> +wildcard_matches(char const *wildcard, char const *str)
>> +{
>> +	switch (*wildcard) {
>> +	case '\0':
>> +		return !*str;
>> +	case '?':
>> +		return *str && wildcard_matches(wildcard + 1, str + 1);
>> +	case '*':
>> +		return (*str && wildcard_matches(wildcard, str + 1))
>> +			|| wildcard_matches(wildcard + 1, str);
>> +	default:
>> +		return (*wildcard == *str)
>> +			&& wildcard_matches(wildcard + 1, str + 1);
>> +	};
>> +}
> 
> Umm, I saw somewhere you #included fnmatch.h. Why not use fnmatch()?

I'll have to check if that is still needed (might not be). I'd started
with fnmatch(), but the behavior does not match the more common unit
test conventions. I'd also checked using regcomp() and at least one other.


>> diff --git a/tools/zunitc/test/fixtures_test.c b/tools/zunitc/test/fixtures_test.c
>> new file mode 100644
>> index 0000000..95af04a
>> --- /dev/null
>> +++ b/tools/zunitc/test/fixtures_test.c
> 
>> +#include "config.h"
>> +
>> +/**
>> + * Tests of fixtures.
>> + */
>> +
>> +#include <ctype.h>
>> +#include <stdlib.h>
>> +#include <stddef.h>
>> +#include <string.h>
>> +
>> +#include "zunitc/zunitc.h"
>> +
>> +
>> +
>> +/* Use a simple string for a simplistic fixture */
>> +static struct zuc_fixture fixture_minimal = {
>> +	.data = "for all good men to",
>> +};
>> +
>> +ZUC_TEST_F(fixture_minimal, just_as_is)
>> +{
>> +	const char *str = data;
> 
> Hmm, could we have all implicit variables mentioned as macro arguments?
> Or some way to show that 'data' is defined by the macro? Or does that
> get too ugly?

I think it might not get too ugly... just would take a little more time
to analyze best implementations, etc. Since this was a point that
already bothered me, your noticing means it's worth that time now.


>> +	ZUC_ASSERT_NOT_NULL(str);
>> +
>> +	ZUC_ASSERT_EQ(0, strcmp("for all good men to", str));
>> +}
>> +
>> +/*
>> + * Not important what or how data is manipulated, just that this function
>> + * does something non-transparent to it.
>> + */
>> +static void *
>> +setup_test_config(void *data)
>> +{
>> +	int i;
>> +	const char *str = data;
>> +	char *upper = NULL;
>> +	ZUC_EXPECT_NOT_NULL(data);
>> +	if (str) {
>> +		upper = strdup(str);
>> +		ZUC_EXPECT_NOT_NULL(upper);
>> +		if (upper) {
>> +			for (i = 0; upper[i]; ++i)
>> +				upper[i] = (char)toupper(upper[i]);
>> +		}
>> +	}
>> +
>> +	return upper;
>> +}
>> +
>> +static void
>> +teardown_test_config(void *data)
>> +{
>> +	ZUC_ASSERT_NOT_NULL(data);
>> +	free(data);
>> +}
>> +
>> +static struct zuc_fixture fixture_data0 = {
>> +	.data = "Now is the time",
>> +	.set_up = setup_test_config,
>> +	.tear_down = teardown_test_config
>> +};
>> +
>> +ZUC_TEST_F(fixture_data0, base)
>> +{
>> +	const char *str = data;
>> +	ZUC_ASSERT_NOT_NULL(str);
>> +
>> +	ZUC_ASSERT_EQ(0, strcmp("NOW IS THE TIME", str));
>> +}
>> +
>> +/* Use the same fixture for a second test. */
>> +ZUC_TEST_F(fixture_data0, no_lower)
>> +{
>> +	int i;
>> +	const char *str = data;
>> +	ZUC_ASSERT_NOT_NULL(str);
>> +
>> +	for (i = 0; str[i]; ++i)
>> +		ZUC_ASSERT_EQ(0, islower(str[i]));
>> +}
> 
> Ahh, so ZUC_TEST_F() *uses* a fixture, it doesn't define one. Why did I
> get the idea it defines one... the doc for ZUC_TEST_F?
> 
> Now I get the idea what the fixtures are and how they are used!

OK. Definitely pinpoints areas that need more clarity in the
documentation. Getting devs to use existing fixtures isn't too hard, but
the subtleties of their creation can take a bit to learn.


>> diff --git a/tools/zunitc/test/zunitc_test.c b/tools/zunitc/test/zunitc_test.c
>> new file mode 100644
>> index 0000000..3a5ad8e
>> --- /dev/null
>> +++ b/tools/zunitc/test/zunitc_test.c
> 
>> +#include "config.h"
>> +
>> +/*
>> + * A simple file to show tests being setup and run.
>> + */
>> +
>> +#include <stddef.h>
>> +#include <stdlib.h>
>> +#include <string.h>
>> +
>> +#include "zunitc/zunitc.h"
>> +
>> +/*
>> + * The SKIP and FAIL sets of tests are those that will cause 'make check'
>> + * to fail so are disabled by default. They can be re-enabled when working
>> + * on the test framework itself.
>> + */
>> +
>> +/* #define ENABLE_FAIL_TESTS */
>> +/* #define ENABLE_SKIP_TESTS */
>> +
>> +ZUC_TEST(base_test, math_is_sane)
>> +{
>> +	ZUC_EXPECT_EQ(4, 2 + 2);
>> +}
> 
> Alright, good to test the testing framework too.
> 
> Ok. There is nothing in the design that makes me scream, that's
> good. :-)
> 
> It does seems a bit complicated at first, but now I have at least a
> hunch on where you are going. The rough direction seems good, based on a
> quick read of this one patch.
> 
> 
> Thanks,
> pq
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
> 

-- 
Jon A. Cruz - Senior Open Source Developer
Samsung Open Source Group
jonc at osg.samsung.com


More information about the wayland-devel mailing list