On 17/01/2024 14:38, Chandra Pratap via GitGitGadget wrote:
From: Chandra Pratap <chandrapratap3519@xxxxxxxxx>
t/t0009-prio-queue.sh along with t/helper/test-prio-queue.c unit
tests Git's implementation of a priority queue. Migrate the
test over to the new unit testing framework to simplify debugging
and reduce test run-time. Refactor the required logic and add
a new test case in addition to porting over the original ones in
shell.
Thanks for working on this, it is good to see people exploring the unit
test framework. I agree with the points that Junio and Peff have already
made, I've got a couple of additional comments below.
> [...]
+static int test_prio_queue(int *input, int *result)
This function does not need to return a value.
+{
+ struct prio_queue pq = { intcmp };
+ int i = 0;
+
+ while (*input) {
I agree with Junio that a for() loop would be better here, but I think
that rather than testing for '0' we should just pass the length of the
input array to this function.
+ int *val = input++;
+ void *peek, *get;
+ switch(*val) {
+ case GET:
+ peek = prio_queue_peek(&pq);
+ get = prio_queue_get(&pq);
+ if (peek != get)
+ BUG("peek and get results don't match");
If possible avoid calling BUG() in unit test code as it aborts the whole
test program, not just the current test. You can use check() and return
early instead
if (!check(peek, !=, get))
return;
+ result[i++] = show(get);
I agree with Peff that it would be safer just to use check_int() here.
If you switch to a for loop with an index you can also print the current
index with
test_msg("index %d", i);
Which will help diagnose which item in the queue is failing.
Alternatively we could grow result dynamically using ALLOC_GROW() and
add a function to compare two arrays of integers. Such a function should
take the two lengths, one for each array.
Best Wishes
Phillip
