[PATCH] DRAFT: Add new test which covers force user bug

Robin Hack rhack at redhat.com
Wed Feb 3 09:31:50 UTC 2016


Hi Uri.

Patch looks great. Test passed and failed as expected. Thank you very
much for rebase and your help.

Maybe I or You should add Pair-programmed-with tag :).

If you feel that patch is right now I would like to ask someone else
to review it.

PS: I agree with proposed changes like: move force user tests to one place.

Have nice day
Robin Hack

On Tue, Feb 2, 2016 at 8:15 PM, Uri Simchoni <uri at samba.org> wrote:
> Robin,
> Here's a slightly modified patch with rebase and my notes applied, let me
> know if that suits you.
>
> Hopefully we'll get all "force user" tests together into a new test suite as
> Andreas suggested.
>
> What I meant by "has to run on all envs" was that the new setup for
> force-user-valid-users testing is only in the fileserver env (as many other
> setups), and samba3.blackbox.smbclient_auth.plain test suite runs on all
> envs. Therefore if we "just" add a line that tries to connect to the new
> share, it would work on fileserver env but fail on other envs.
>
> Thanks,
> Uri.
>
>
> On 02/02/2016 04:37 PM, Robin Hack wrote:
>>
>> Hi Uri and Andreas.
>>
>> Thank you for your feedback. I must admit that I didn't explore samba
>> testsuite fully.
>>
>> I agree that source3/script/tests/test_smbclient_auth.sh is much
>> better place to live :).
>>
>> However.
>>
>> I don't think that add one line to
>> samba3.blackbox.smbclient_auth.plain would work. I really need to use
>> group with exactly same name as user have. I created one user and
>> group named: force_user.
>> Should I hardcode user name and group name to test-shell script?
>> Or extend parameters of script?
>>
>> Uri, what do you mean by: "it has to run on all envs."?
>>
>> Patch need rebase because new tests were added :).
>>
>> Thank you and have nice day
>> Robin Hack
>>
>> On Mon, Feb 1, 2016 at 7:17 PM, Uri Simchoni <uri at samba.org> wrote:
>>>
>>> Yeah, OK.
>>> samba3.blackbox.smbclient_auth.plain seemed like a place where adding the
>>> test would be just adding a line, but it won't work - it has to run on
>>> all
>>> envs. samba3.blackbox.valid_users is not where we want it - it should be
>>> with other "force user" tests eventually.
>>>
>>> The patch does need the other fixes I pointed out, and re-basing (I tried
>>> doing the fixes myself but due to the rebase need, I cannot apply it with
>>> git am and applying with "patch" would lose all the gitness of the patch
>>> :))
>>>
>>> Thanks,
>>> Uri.
>>>
>>>
>>> On 02/01/2016 12:18 PM, Andreas Schneider wrote:
>>>>
>>>> On Wednesday 27 January 2016 13:17:04 Uri Simchoni wrote:
>>>>>
>>>>> It appears to me that the essence of this new test suite is to perform
>>>>> a
>>>>> successful tree-connect to the force_user_valid_users share, using
>>>>> $USERNAME/$PASSWORD credentials. Can't it be just added as another test
>>>>> to an existing test suite that tests similar things using similar
>>>>> techniques? Good candidates are:
>>>>> - samba3.blackbox.valid_users
>>>>> - samba3.blackbox.smbclient_auth.plain
>>>>>
>>>>> (actually I think the two test suites should be merged, or that all
>>>>> "force user" test should be combined in a separate test suite, where it
>>>>> can be verified not only that the tree-connect succeeds, but also that
>>>>> the force-user works, e.g. by creating a file and doing smbcacls on it
>>>>> -
>>>>> but that can be done later. In the meantime I don't see a reason for
>>>>> adding another test suite)
>>>>
>>>> I think we should move all 'force user' test to a new test. Could we
>>>> push
>>>> the
>>>> patch upstream and then work on merging them?
>>>>
>>>>
>>>>          -- andreas
>>>>
>



More information about the samba-technical mailing list