Looks good overall. I attached a new version with a few small changes:
> * Changed store_conn_addrinfo to return int like how all the functions
> dealing with addrinfo does. Also moved the error reporting to inside there
> where the error happened.
I don't feel strong about the int vs bool return type. The existing
static libpq functions are a bit of a mixed bag around this, so either
way seems fine to me. And moving the log inside the function seems
fine too. But it seems you accidentally removed the "goto
error_return" part as well, so now we're completely ignoring the
allocation failure. The attached patch fixes that.
>+ok($node1_occurences > 1, "expected at least one execution on node1, found none");
>+ok($node2_occurences > 1, "expected at least one execution on node2, found none");
>+ok($node3_occurences > 1, "expected at least one execution on node3, found none");
I changed the message to be a description of the expected case,
instead of the failure case. This is in line with the way these
messages are used in other tests, and indeed seems like the correct
way because you get output from "meson test -v postgresql:libpq /
libpq/003_load_balance_host_list" like this:
▶ 6/6 - received at least one connection on node1 OK
▶ 6/6 - received at least one connection on node2 OK
▶ 6/6 - received at least one connection on node3 OK
▶ 6/6 - received 50 connections across all nodes OK
Finally, I changed a few small typos in your updated commit message
(some of which originated from my earlier commit messages)
