[3/4] secur32/tests: Add a test for QueryCredentialsAttributes(SECPKG_CRED_ATTR_NAMES). Take 2.

Dmitry Timoshkov dmitry at baikal.ru
Thu Dec 12 01:10:22 CST 2013


Nikolay Sivov <bunglehead at gmail.com> wrote:

> >>>>> +    st = pQueryCredentialsAttributesA(&cred, SECPKG_CRED_ATTR_NAMES, &names);
> >>>>> +    ok(st == SEC_E_NO_CREDENTIALS || st == SEC_E_UNSUPPORTED_FUNCTION /* before Vista */, "expected SEC_E_NO_CREDENTIALS, got %08x\n", st);
> >>>>> +
> >>>> Second possible return code should be broken(), otherwise your code
> >>>> change in next patch won't make any effect on tests.
> >>> SEC_E_UNSUPPORTED_FUNCTION is a perfectly valid error code, broken is not
> >>> appropriate here. My next patch just changes Wine code to follow more recent
> >>> Windows behaviour.
> >>>
> >> broken() is a way to mark it as undesired code for wine implementation,
> >> it doesn't necessary mean that something
> >> changed significantly from error to success and such. Old return code
> >> will remain valid when running on windows.
> > That's just another way of action.
> I'm just pointing out that a particular improvement could be made to 
> ensure code change is actually tested, that's all.

Thanks, I believe that I've answered that part.

> >   There are numerous places in Wine
> > tests that accept several different error codes and none of them is
> > marked as broken, my patch just does the same thing.
> >
> Well, that's irrelevant, there's some places where it's done like this 
> and some where it's not.

And what is the difference between those places? Isn't that up to the patch
author or there are some technical reasons?

> But I get that you're not willing to change a patch, so I'll just wait 
> for possible comments from others.

My impression was that you were trying to clarify the reason why my patch
doesn't use broken() and see my explanation, but now I see that actual
reason was to "prove" my "willingless" to change the patches I send.

-- 
Dmitry.



More information about the wine-devel mailing list