"Linus Arver via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
> +UNIT_TEST_PROGRAMS += t-trailer
> UNIT_TEST_PROGS = $(patsubst %,$(UNIT_TEST_BIN)/%$X,$(UNIT_TEST_PROGRAMS))
> UNIT_TEST_OBJS = $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(UNIT_TEST_PROGRAMS))
> UNIT_TEST_OBJS += $(UNIT_TEST_DIR)/test-lib.o
Totally offtopic, but does it bother folks who are interested in
adding more unit tests that they do not seem to interact very well
with GIT_SKIP_TESTS environment variable?
> diff --git a/t/unit-tests/t-trailer.c b/t/unit-tests/t-trailer.c
> new file mode 100644
> index 00000000000..147a51b66b9
> --- /dev/null
> +++ b/t/unit-tests/t-trailer.c
> @@ -0,0 +1,175 @@
> +#include "test-lib.h"
> +#include "trailer.h"
> +
> +static void t_trailer_iterator(const char *msg, size_t num_expected_trailers)
> +{
> + struct trailer_iterator iter;
> + size_t i = 0;
> +
> + trailer_iterator_init(&iter, msg);
> + while (trailer_iterator_advance(&iter)) {
> + i++;
> + }
Unnecessary {braces} around a single-statement block?
> + trailer_iterator_release(&iter);
> +
> + check_uint(i, ==, num_expected_trailers);
> +}
> +
> +static void run_t_trailer_iterator(void)
> +{
> + static struct test_cases {
> + const char *name;
> + const char *msg;
> + size_t num_expected_trailers;
This is more like number of lines in the trailer block, not
limiting its count only to true trailers, no?
> + } tc[] = {
> ...
> + {
> + "with non-trailer lines in trailer block",
> + "subject: foo bar\n"
> + "\n"
> + /*
> + * Even though this trailer block has a non-trailer line
> + * in it, it's still a valid trailer block because it's
> + * at least 25% trailers and is Git-generated.
> + */
> + "not a trailer line\n"
> + "not a trailer line\n"
> + "not a trailer line\n"
> + "Signed-off-by: x\n",
> + 1
> + },
It is OK to leave it num_expected_trailers in this step and then
rename it when you update this "1" (number of real trailer lines)
to "4" (number of lines in the trailer block).
I wonder if you'd want to make more data available to the test. At
least it would be more useful if the number of true trailer lines
and the number of lines in the trialer block are available
separately.
The interface into the trailers that is being tested by this code is
"the caller repeatedly calls the iterator, and the caller can
inspect the iterator's state available as its .raw, .key and .val
members and use them as it sees fit", so you could check, if you
wanted to, the following given the above sample data:
* the first iteration finds no key/value pair (optionally, the
contents found in the .raw member is as expected).
* the second iteration finds no key/value pair (ditto).
* the third iteration finds no key/value pair (ditto).
* the fourth iteration finds key="Signed-off-by" value="x".
* there is no fifth iteration.
but the current code only checks the last condition and nothing
else. I dunno.
