[PATCH] Some tests for lsa

[Julius Schwartzenberg]

Hi Bruno,

Bruno Jesus wrote:
> On Sun, Feb 5, 2017 at 5:57 PM, Julius Schwartzenberg
> <julius.schwartzenberg at gmail.com> wrote:
>> ---
>>  dlls/secur32/tests/Makefile.in |   1 +
>>  dlls/secur32/tests/lsa.c       | 215 +++++++++++++++++++++++++++++++++++++++++
>>  include/ntsecapi.h             |  38 ++++++++
>>  3 files changed, 254 insertions(+)
>>  create mode 100644 dlls/secur32/tests/lsa.c
> 
> Hi, thanks for the patch. Unfortunately as is it cannot be used.
> Somethings are stated below but mostly you have to check the compiler
> warnings and guarantee there is none. Also, although very minor, git
> apply complains about 7 lines adding trailing whitespaces.

Thanks a lot for the review! I hope to send a better version soon. I
just wasn't sure whether wine-patches was already appropriate for
non-final patches or not. It seems nice that Marvin runs the tests already.
Let me know if I'd better send it differently before the review process
is finished.

>> + * Lsa tests
> 
> The copyright line is missing, example:
> 
> Copyright 2017 Julius Schwartzenberg

I wasn't sure whether this was important, but I can add that.


>> +static void test_LsaLookupAuthenticationPackage_correct_handle(void)
>> +{
>> +    NTSTATUS status;
>> +    HANDLE lsa_handle;
>> +    LSA_STRING package_name;
>> +    ULONG auth_package;
>> +    package_name.Buffer = package_name_kerberos;
>> +    package_name.Length = (USHORT)strlen(package_name.Buffer);
>> +
>> +    LsaConnectUntrusted(&lsa_handle);
>> +    status = LsaLookupAuthenticationPackage(lsa_handle, &package_name, &auth_package);
>> +
>> +    ok(status == STATUS_SUCCESS,
>> +      "LsaLookupAuthenticationPackage(valid_handle, \"Kerberos\", ...) returned 0x%x\n", status);
> 
> Is it possible to test that auth_package somehow?

Possibly yeah. I haven't figured out much yet. I just started to write a
bunch of tests some time ago to see whether I could figure things out. I
mailed it now because it seemed better to submit it after the 2.0
release. Would it make sense to add this test in a second patch?

These tests so far are really really basic and it seemed to make sense
to me to add more tests in later patches. (Or first solve some todo_wines?)


>> diff --git a/include/ntsecapi.h b/include/ntsecapi.h
>> index 2bb3d31..439f67e 100644
>> --- a/include/ntsecapi.h
>> +++ b/include/ntsecapi.h
>> @@ -387,6 +387,44 @@ NTSTATUS WINAPI LsaSetTrustedDomainInformation(LSA_HANDLE,PSID,TRUSTED_INFORMATI
>>  NTSTATUS WINAPI LsaStorePrivateData(LSA_HANDLE,PLSA_UNICODE_STRING,PLSA_UNICODE_STRING);
>>  NTSTATUS WINAPI LsaUnregisterPolicyChangeNotification(POLICY_NOTIFICATION_INFORMATION_CLASS,HANDLE);
>>
>> +#ifndef MICROSOFT_KERBEROS_NAME_A
>> +
>> +#define MICROSOFT_KERBEROS_NAME_A "Kerberos"
>> +#define MICROSOFT_KERBEROS_NAME_W L"Kerberos"
> 
> You can't use L to define WCHAR data.
> 
>> +#ifdef WIN32_CHICAGO
> 
> Where is this from? Not a single Wine header has that name.

I copy pasted the headers from MinGW. On IRC I was told that is ok
because the file starts with "This file has no copyright assigned and is
placed in the Public Domain.".

I didn't consider any technical issues though. How should MinGW/Wine
header differences be dealt with?

Thanks again for the feedback! I hope I can send a better patch soon.

Best regards,
Julius