From: "Rémi Bernon" Subject: Re: [PATCH v3 4/6] ntoskrnl.exe/tests: Add some HidP_Get*Caps tests. Message-Id: Date: Tue, 8 Jun 2021 23:10:32 +0200 In-Reply-To: <6360e559-d4a7-7298-0246-e0919221a543@codeweavers.com> References: <20210604091220.265735-1-rbernon@codeweavers.com> <20210604091220.265735-4-rbernon@codeweavers.com> <6360e559-d4a7-7298-0246-e0919221a543@codeweavers.com> On 6/8/21 10:59 PM, Zebediah Figura (she/her) wrote: >> exp->NumberFeatureDataIndices, "unexpected caps >> NumberFeatureDataIndices %d, expected %d\n", >> caps->NumberFeatureDataIndices, exp->NumberFeatureDataIndices); >> +} > > These are some *really* long lines, and same with the ones below. > > I guess it's always nice to see what exactly differs, but maybe it's > more worthwhile just to use memcmp()? I don't feel strongly about it, > though. > I think memcmp is fine up to the moment where the test breaks. To debug the issue it's nice to see what didn't match without having to write those long lines yourself (same for debugstr BTW, I'd love to have more helpers to dump the various Win32 structs readily available). And for instance I don't like the report memcmp very much, because it doesn't tell you at all what's wrong with HidP_InitializeReportForID. This only needs to be written once and (hopefully) nobody will have to look at it again. But then you can use it to check that both struct match and have precise info when they don't. I would have like to be able to put individual todo_wine to replace the additional tests for the partially matching structs, but it was not convenient, so instead I'm just going to replace all the checks with a single call to these functions when the todo_wine are fixed. I can wrap the lines though ;) -- Rémi Bernon