On 6/8/21 4:37 PM, Rémi Bernon wrote:
On 6/8/21 11:35 PM, Zebediah Figura (she/her) wrote:
On 6/8/21 4:33 PM, Rémi Bernon wrote:
On 6/8/21 11:27 PM, Zebediah Figura (she/her) wrote:
On 6/8/21 4:10 PM, Rémi Bernon wrote:
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.
Yeah, though on the other hand that's one reason it's nice to use memcmp - either they match or they don't.
Just a bit of extra 2¢: in DirectShow I ended up using memcmp() to match AM_MEDIA_TYPE. If a test breaks, which is not infrequently, I temporarily add strmbase_dump_media_type() to check the difference. Actually I've considered adding an automatic helper like that to compare_media_types() everywhere, though it'd be nice to have a debugstr_* type helper instead so that I don't have to turn on +strmbase to use it.
It may be a nice approach in general to structure things like
static bool compare_some_struct(const SOME_STRUCT *s, const SOME_STRUCT *expect) { if (!memcmp(s, expect, sizeof(*SOME_STRUCT))) return true; trace("Expected: %s\n", debugstr_some_struct(expect)); trace("Received: %s\n", debugstr_some_struct(s)); }
...
{ ok(compare_some_struct(&s, &expect), "Structures didn't match.\n"); }
I don't have good opinion of trace-ing failures, I find that it quickly gets cut, and in general the traces you were interested it are after the cut.
Sorry, I'm not sure I understand what you mean by this; can you please clarify?
Eh sorry for my bad english, I mean that traces are muted after a being printed too many times, and it happens too often when I needed the traces.
Yes, I find the choice of defaults is a bit unfortunate, I'd rather see muting off by default, and enabled only for things like the testbot where we really care about report length.