[RESEND PATCH] DsBind and DsGetDomainControllerInfo to return 2k8 structs complete

[Garming Sam]

Seeing that that's already there, extending that test seems more
appropriate. If you are going to write a torture test, you want to try
split out each of your cases into actual separate tests as
torture_tcase_add_simple_test which is at the end of the file, so you
know which ones are failing individually. Whichever you're more
comfortable with, I guess.

Cheers.


On 21/04/16 07:15, voidswitch wrote:
> Hi,
>
> Currently I've done some cleanup, and look into writing tests. Is it
> preferable to write them in Python, or should it be added to
> source4/torture/rpc/drsuapi.c?
>
> regards,
>
> Dirk
>
> On 19.04.2016 23:45, Garming Sam wrote:
>> Hi,
>>
>> I see what you mean now, so that all seems fine. If you haven't already
>> found it, you might want to look at the definition of drs_DsBind in
>> python/samba/drs_utils.py for the tests.
>>
>> Cheers.
>>
>> On 19/04/16 17:08, voidswitch wrote:
>>> Hi,
>>>
>>> On 19.04.2016 00:56, Garming Sam wrote:
>>>> Hi Dirk,
>>>>
>>>> Just from looking a the patch, I've noticed that there was a info24
>>>> that
>>>> used to be handled as well as the info28 (so a case statement with the
>>>> three would be preferable). I believe there are some other lengths
>>>> also
>>>> available (probably for 2012), but we probably don't handle them
>>>> properly in other places either so you probably don't need to worry
>>>> about those (unless you really want to go out of your way).
>>> drsuapi_bind_state used to store info28 structs only, so any other
>>> size needed to be converted. This was done only with info24 requests.
>>> I changed drsuapi_bind_state to use DsBindInfoCtr. Whatever the client
>>> sends, we simply can store it in drsuapi_bind_state. Same with our
>>> responses.
>>>
>>> Our response should probably take the current functional level into
>>> account. Currently I send a response which clients should understand,
>>> as length of the request >= length of the response. Clients sending 24
>>> requests have always gotten a 28 response.
>>>> More importantly though, a test would be good to assert that the
>>>> changes
>>>> haven't broken anything (and test your new behaviour). A good way
>>>> to do
>>>> this would be to order the patches so that first there is the test
>>>> which
>>>> passes against the existing levels and fails against the new one. And
>>>> then would be the patch that fixes the behaviour for the new case
>>>> so all
>>>> the tests now pass.
>>>>
>>>> An example of how you might start:
>>>> source4/torture/drs/python/getnc_exop.py
>>>>
>>>> You'd need to do the bind at the different levels and basically just
>>>> assert that the result values are how you would expect. It should be a
>>>> straightforward test, but a valuable one.
>>> This should push me in the right direction regarding tests. I'll add
>>> them.
>>>> Some minor things:
>>>> * There's a few C++ '//' comments, which we try to avoid.
>>>> * The for loop is a little inconsistent, there's spaces for i <
>>>> res->count, but not for i = 0. And I now realize that it was copied
>>>> from
>>>> above... It would be nice to be able to share some part of this
>>>> code if
>>>> possible.
>>> The copy was the simplest option available, since it is really the
>>> same. I guess a patch to clean this up should follow. Anyway I'll look
>>> at it.
>>>> If you haven't already read the README.Coding, it's a good place to
>>>> start. Otherwise, the changes seem quite reasonable and thanks for
>>>> working on it. If you need any other help, feel free to ask.
>>> Thanks for the comments,
>>> Dirk
>>>
>>>> Cheers,
>>>>
>>>> Garming
>>>>
>>>> On 19/04/16 09:20, Void wrote:
>>>>> I'll get this right eventually.
>>>>>
>>>>> Thanks
>>>>> Dirk
>