[PATCH] test samba-tool --help

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] test samba-tool --help

Samba - samba-technical mailing list
Running `samba-tool --help` lists a number of subcommands, and if you
recurse into the help tree with a subcommand like so

$ bin/samba-tool spn --help

you are given a list of sub-subcommands.

This patch climbs the samba-tool --help tree and checks that each call
succeeds. Incidentally, there are 109 nodes on the tree.

Why bother, you are probably asking, given this test is doing very
little and exercising the same bit of code 109 times. Well, there are
two reasons:

1) I want to change some things in the help code.

2) as far as I can tell, some branches (like "spn") are not tested in
any way. This test will at least catch syntax errors.

cheers,
Douglas

0001-samba-tool-help-test-ensuring-help-tree-coverage.patch (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] test samba-tool --help

Samba - samba-technical mailing list
On Mon, 2017-12-18 at 17:37 +1300, Douglas Bagnall via samba-technical
wrote:

> Running `samba-tool --help` lists a number of subcommands, and if you
> recurse into the help tree with a subcommand like so
>
> $ bin/samba-tool spn --help
>
> you are given a list of sub-subcommands.
>
> This patch climbs the samba-tool --help tree and checks that each call
> succeeds. Incidentally, there are 109 nodes on the tree.
>
> Why bother, you are probably asking, given this test is doing very
> little and exercising the same bit of code 109 times. Well, there are
> two reasons:
>
> 1) I want to change some things in the help code.
>
> 2) as far as I can tell, some branches (like "spn") are not tested in
> any way. This test will at least catch syntax errors.

G'Day Douglas,

This looks good, but why didn't you use self.check_output() from BlackBoxTestCase?

This finds the BINDIR without needing the environment by using the python file location.

Fix that up and a review will be easy to obtain :-)

Thanks,

Andrew Bartlett
--
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] test samba-tool --help

Samba - samba-technical mailing list
On 18/12/17 21:13, Andrew Bartlett via samba-technical wrote:

> On Mon, 2017-12-18 at 17:37 +1300, Douglas Bagnall via samba-technical
> wrote:
>> Running `samba-tool --help` lists a number of subcommands, and if you
>> recurse into the help tree with a subcommand like so
>>
>> $ bin/samba-tool spn --help
>>
>> you are given a list of sub-subcommands.
>>
>> This patch climbs the samba-tool --help tree and checks that each call
>> succeeds. Incidentally, there are 109 nodes on the tree.
>>
>> Why bother, you are probably asking, given this test is doing very
>> little and exercising the same bit of code 109 times. Well, there are
>> two reasons:
>>
>> 1) I want to change some things in the help code.
>>
>> 2) as far as I can tell, some branches (like "spn") are not tested in
>> any way. This test will at least catch syntax errors.
>
> G'Day Douglas,
>
> This looks good, but why didn't you use self.check_output() from BlackBoxTestCase?
>
> This finds the BINDIR without needing the environment by using the python file location.
>
> Fix that up and a review will be easy to obtain :-)
Here you go, though I don't really like how that function insists on
passing arguments through a shell when there is no need.

cheers,

Douglas

0001-samba-tool-help-test-ensuring-help-tree-coverage.patch (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] test samba-tool --help

Samba - samba-technical mailing list
G'Day Douglas,

You still look up BINDIR with this line:

On Tue, 2017-12-19 at 21:30 +1300, Douglas Bagnall via samba-technical
wrote:
>        samba_tool = os.path.join(os.environ['BINDIR'], 'samba-tool')

Sorry,

Andrew Bartlett

--
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] test samba-tool --help

Samba - samba-technical mailing list
On 20/12/17 21:41, Andrew Bartlett via samba-technical wrote:
> G'Day Douglas,
>
> You still look up BINDIR with this line:
>
> On Tue, 2017-12-19 at 21:30 +1300, Douglas Bagnall via samba-technical
> wrote:
>>        samba_tool = os.path.join(os.environ['BINDIR'], 'samba-tool')
>

Oh. It seems so. But look NOW.

Douglas

0001-samba-tool-help-test-ensuring-help-tree-coverage.patch (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] test samba-tool --help

Samba - samba-technical mailing list
On Thu, 2017-12-21 at 11:08 +1300, Douglas Bagnall via samba-technical
wrote:

> On 20/12/17 21:41, Andrew Bartlett via samba-technical wrote:
> > G'Day Douglas,
> >
> > You still look up BINDIR with this line:
> >
> > On Tue, 2017-12-19 at 21:30 +1300, Douglas Bagnall via samba-technical
> > wrote:
> > >        samba_tool = os.path.join(os.environ['BINDIR'], 'samba-tool')
>
> Oh. It seems so. But look NOW.
>
> Douglas

Reviewed-by: Andrew Bartlett <[hidden email]>

Please push!

Thanks,

Andrew Bartlett

--
Andrew Bartlett
https://samba.org/~abartlet/
Authentication Developer, Samba Team         https://samba.org
Samba Development and Support, Catalyst IT  
https://catalyst.net.nz/services/samba