From: Phillip Wood <phillip.wood123@gmail.com>
To: steadmon@google.com, git@vger.kernel.org
Cc: calvinwan@gmail.com, szeder.dev@gmail.com, chooglen@google.com,
avarab@gmail.com, gitster@pobox.com,
sandals@crustytoothpaste.net, Calvin Wan <calvinwan@google.com>
Subject: Re: [PATCH RFC v2 4/4] unit test: add basic example and build rules
Date: Thu, 18 May 2023 14:32:51 +0100 [thread overview]
Message-ID: <e26789b2-21fd-668d-ee00-2640d5b8d5c2@gmail.com> (raw)
In-Reply-To: <20230517-unit-tests-v2-v2-4-21b5b60f4b32@google.com>
On 18/05/2023 00:56, steadmon@google.com wrote:
> Integrate a simple strbuf unit test with Git's Makefiles.
>
> You can build and run the unit tests with `make unit-tests` (or just
> build them with `make build-unit-tests`). By default we use the basic
> test runner from the C-TAP project, but users who prefer prove as a test
> runner can set `DEFAULT_UNIT_TEST_TARGET=prove-unit-tests` instead.
>
> We modify the `#include`s in the C TAP libraries so that we can build
> them without having to include the t/ directory in our include search
> path.
Thanks for adding some example tests, it is really helpful to see how
the library will be used.
I tried building the units test with SANITIZE=address set and I get lots
of link errors complaining about undefined references to __asan_*
> Signed-off-by: Calvin Wan <calvinwan@google.com>
> Signed-off-by: Josh Steadmon <steadmon@google.com>
> Change-Id: Ie61eafd2bd8f8dc5b30449af1e436889f91da3b7
> diff --git a/t/strbuf-test.c b/t/strbuf-test.c
> new file mode 100644
> index 0000000000..8f8d4e11db
> --- /dev/null
> +++ b/t/strbuf-test.c
> @@ -0,0 +1,54 @@
> +#include "tap/basic.h"
> +
> +#include "../git-compat-util.h"
> +#include "../strbuf.h"
> +
> +int strbuf_init_test()
> +{
> + struct strbuf *buf = malloc(sizeof(void*));
Is there a reason to use dynamic allocation here. Also I think you need
sizeof(*buf) to allocate the correct size.
> + strbuf_init(buf, 0);
> +
> + if (buf->buf[0] != '\0')
> + return 0;
> + if (buf->alloc != 0)
> + return 0;
> + if (buf->len != 0)
> + return 0;
> + return 1;
> +}
This test nicely illustrates why I'd prefer a different approach. The
test author has to maintain the pass/fail state and there are no
diagnostics if it fails to tell you which check failed. To be clear I
view the lack of diagnostics as the fault of the test framework, not the
test author. I'd prefer something like
void strbuf_init_test(void)
{
struct strbuf buf;
strbuf_init(&buf, 0);
check_char(buf.buf[0] == '\0');
check_uint(buf.alloc, ==, 0);
check_uint(buf.len, ==, 0);
}
which would be run as
TEST(strbuf_init_test(), "strbuf_init initializes properly");
in main() and provide diagnostics like
# check "buf.alloc == 0" failed at my-test.c:102
# left: 2
# right: 0
when a check fails.
> +int strbuf_init_test2() {
> + struct strbuf *buf = malloc(sizeof(void*));
> + strbuf_init(buf, 100);
> +
> + if (buf->buf[0] != '\0')
> + return 0;
> + if (buf->alloc != 101)
Strictly speaking I think the API guarantees that at least 100 bytes
will be allocated, not the exact amount as does alloc_grow() below.
> + return 0;
> + if (buf->len != 0)
> + return 0;
> + return 1;
> +}
> +
> +
> +int strbuf_grow_test() {
> + struct strbuf *buf = malloc(sizeof(void*));
> + strbuf_grow(buf, 100);
> +
> + if (buf->buf[0] != '\0')
> + return 0;
> + if (buf->alloc != 101)
> + return 0;
> + if (buf->len != 0)
> + return 0;
> + return 1;
> +}
Best Wishes
Phillip
next prev parent reply other threads:[~2023-05-18 13:33 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-17 23:56 [PATCH RFC v2 0/4] Add an external testing library for unit tests steadmon
2023-05-17 23:56 ` [PATCH RFC v2 1/4] common-main: split common_exit() into a new file steadmon
2023-05-18 17:17 ` Junio C Hamano
2023-07-14 23:38 ` Splitting common-main (Was: Re: [PATCH RFC v2 1/4] common-main: split common_exit() into a new file) Josh Steadmon
2023-07-15 0:34 ` Splitting common-main Junio C Hamano
2023-08-14 13:09 ` Splitting common-main (Was: Re: [PATCH RFC v2 1/4] common-main: split common_exit() into a new file) Jeff Hostetler
2023-05-17 23:56 ` [PATCH RFC v2 2/4] unit tests: Add a project plan document steadmon
2023-05-18 13:13 ` Phillip Wood
2023-05-18 20:15 ` Glen Choo
2023-05-24 17:40 ` Josh Steadmon
2023-06-01 9:19 ` Phillip Wood
2023-05-17 23:56 ` [PATCH RFC v2 3/4] Add C TAP harness steadmon
2023-05-18 13:15 ` Phillip Wood
2023-05-18 20:50 ` Josh Steadmon
2023-05-17 23:56 ` [PATCH RFC v2 4/4] unit test: add basic example and build rules steadmon
2023-05-18 13:32 ` Phillip Wood [this message]
2023-06-09 23:25 ` [RFC PATCH v3 0/1] Add a project document for adding unit tests Josh Steadmon
2023-06-09 23:25 ` [RFC PATCH v3 1/1] unit tests: Add a project plan document Josh Steadmon
2023-06-13 22:30 ` Junio C Hamano
2023-06-30 22:18 ` Josh Steadmon
2023-06-29 19:42 ` Linus Arver
2023-06-29 20:48 ` Josh Steadmon
2023-06-30 19:31 ` Linus Arver
2023-07-06 18:24 ` Glen Choo
2023-07-06 19:02 ` Junio C Hamano
2023-07-06 22:48 ` Glen Choo
2023-06-30 21:33 ` Josh Steadmon
2023-06-29 21:21 ` Junio C Hamano
2023-06-30 0:11 ` Linus Arver
2023-06-30 14:07 ` Phillip Wood
2023-06-30 18:47 ` K Wan
2023-06-30 22:35 ` Josh Steadmon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e26789b2-21fd-668d-ee00-2640d5b8d5c2@gmail.com \
--to=phillip.wood123@gmail.com \
--cc=avarab@gmail.com \
--cc=calvinwan@gmail.com \
--cc=calvinwan@google.com \
--cc=chooglen@google.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=phillip.wood@dunelm.org.uk \
--cc=sandals@crustytoothpaste.net \
--cc=steadmon@google.com \
--cc=szeder.dev@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).