From: Jinoh Kang Subject: Re: [PATCH] ws2_32/tests: Add order-agnostic check in test_simultaneous_async_recv. Message-Id: <66b7ec22-2ec6-a744-e03f-c29a09d8e012@gmail.com> Date: Mon, 24 Jan 2022 22:42:12 +0900 In-Reply-To: References: <684917dd-81d8-12a8-1f27-7ce8ed6bacbe@gmail.com> On 1/11/22 06:00, Zebediah Figura (she/her) wrote: > On 1/9/22 06:26, Jinoh Kang wrote: >> On 1/9/22 03:37, Zebediah Figura (she/her) wrote: >>> On 1/7/22 12:15, Jinoh Kang wrote: >>>> Signed-off-by: Jinoh Kang >>>> --- >>>>    dlls/ws2_32/tests/sock.c | 16 ++++++++++++++++ >>>>    1 file changed, 16 insertions(+) >>>> >>>> diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c >>>> index 782b1e59729..966a681e809 100644 >>>> --- a/dlls/ws2_32/tests/sock.c >>>> +++ b/dlls/ws2_32/tests/sock.c >>>> @@ -11520,6 +11520,8 @@ static void test_simultaneous_async_recv(void) >>>>            const void *expect = msgstr + i * stride; >>>>            const void *actual = resbuf + i * stride; >>>>            DWORD size; >>>> +        int allcmp; >>>> +        size_t j; >>>>              ret = WaitForSingleObject(events[i], 1000); >>>>            ok(!ret, "wait timed out\n"); >>>> @@ -11528,6 +11530,20 @@ static void test_simultaneous_async_recv(void) >>>>            ret = GetOverlappedResult((HANDLE)client, &overlappeds[i], &size, FALSE); >>>>            ok(ret, "got error %u\n", GetLastError()); >>>>            ok(size == stride, "got size %u\n", size); >>>> + >>>> +        allcmp = 0; >>>> +        for (j = 0; j <= num_io * stride - size; j++) allcmp |= !memcmp(msgstr + j, actual, size); >>>> +        ok(allcmp, "returned data shall be part of original message (got %s)\n", debugstr_an(actual, size)); >>>> + >>> >>> I'm sorry, but this test makes no sense to me. What exactly are you trying to check here? >> >> It's a redundant check meant to ease debugging in case a future regression causes wine to >> reorder overlapped I/O operations while still completing all of them. >> >> It can be taken out if it is deemed unnecessary.  Actually, the point of this patch is in >> the added comment. >> > > I'm not sure I see the need for this additional test. If we complete the asyncs out of order we'll already see the original message dumped out of order; the existing test already uses debugstr_an(). Fair point, if we don't need the extra test then I guess we can mark it rejected. -- Sincerely, Jinoh Kang