Re: [PATCH v3 4/4] ws2_32/tests: Add tabular socket option validity tests

[Zebediah Figura (she/her)]

On 8/16/21 11:58 PM, Alex Henrie wrote:
> Signed-off-by: Alex Henrie
>
> v3:
> - Replace BROKEN flag with blanket broken case
> - Only test the options in the tables
> - Add IP(V6)_(ADD|DROP)_MEMBERSHIP to the tables
> - Remove undocumented options from the tables
> - Add socket option -1 (always nonexistent) to the tables
>
> dlls/ws2_32/tests/sock.c | 235 +++++++++++++++++++++++++++++++++------
>   1 file changed, 204 insertions(+), 31 deletions(-)

Looks a lot better, thanks. Since I've already added a comment to patch 
3 I'll just add one here as well:
> diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c
> index 51aa0e61282..f60c9d3c75d 100644
> --- a/dlls/ws2_32/tests/sock.c
> +++ b/dlls/ws2_32/tests/sock.c
> @@ -11429,45 +11429,218 @@ static void test_so_debug(void)
>       closesocket(s);
>   }
>
> -static void test_set_only_options(void)
> +struct sockopt_validity_test
>   {
> - unsigned int i;
> - int ret, len;
> - int value;
> - SOCKET s;
> - int opt;
> - int get_error;
> - BOOL get_todo;
> - int set_error;
> - BOOL set_todo;
>
> +};
> - static const struct
>
> +static void do_sockopt_validity_tests(const char *type, SOCKET sock, int level,
> -
>                                  const struct sockopt_validity_test *tests)
>
> +{
> - char value[256];
> - int count, rc, expected_rc, i;
> -
> - for (i = 0; tests[i].opt; i++)
>  {
> -
>    int level;
> -
>    int option;
> -
>    memset(value, 0, sizeof(value));
> -
>    count = sizeof(value);
> -
> -
>    WSASetLastError(0);
> -
>    rc = getsockopt(sock, level, tests[i].opt, value, &count);
> -
>    expected_rc = tests[i].get_error ? SOCKET_ERROR : 0;
>
> +todo_wine_if(!tests[i].get_error && tests[i].get_todo)
> -
>    ok(rc == expected_rc || broken(rc == SOCKET_ERROR && WSAGetLastError() == WSAENOPROTOOPT),
> -
>       "expected getting %s option %i to return %i, got %i\n",
> -
>       type, tests[i].opt, expected_rc, rc);

You could consider simplifying these messages a bit with 
winetest_push_context()/winetest_pop_context().