[PATH] s4-tests: extend LoadParam handling in tests

[Kamen Mazdrashki]

Hi Jelmer,

Sorry for sloow reply, I am a bit on the road now

On Tue, Dec 2, 2014 at 1:42 PM, Jelmer Vernooij <jelmer at samba.org> wrote:

> On Tue, Dec 02, 2014 at 01:33:44PM +0100, Kamen Mazdrashki wrote:
> > On Tue, Dec 2, 2014 at 1:09 PM, Jelmer Vernooij <jelmer at samba.org>
> wrote:
> > > On Tue, Dec 02, 2014 at 05:23:44AM +0100, Kamen Mazdrashki wrote:
> > > > Can you please take a look at attached patch.
> > > > It extends TestCase.get_loadparm() functionality to:
> > > > * throw KeyError in case "SMB_CONF_PATH" is not found in environment
> > > > * load LoadParm load_default() in case function client requests
> default
> > > > handling
> > > >
> > > > Patches in github:
> > > >
> > > >
> > >
> https://github.com/kamenim/samba/commit/ae2bf19d8b52c72ded4ded424e8fd1180133dc81
> > > >
> > > >
> > >
> https://github.com/kamenim/samba/commit/8758d8a2f383f70341a1f059a3944654c8d22339
> > > >
> > > > From ae2bf19d8b52c72ded4ded424e8fd1180133dc81 Mon Sep 17 00:00:00
> 2001
> > > > From: Kamen Mazdrashki <kamenim at samba.org>
> > > > Date: Tue, 2 Dec 2014 05:04:40 +0100
> > > > Subject: [PATCH 1/2] s4-tests/env_loadparm: Throw KeyError in case
> > > >  SMB_CONF_PATH
> > > >
> > > > A bit more specific for the caller to "know" that env key is missing
> > > >
> > > > Change-Id: I4d4c2121af868d79f46f865f420336222bc67347
> > > > Signed-off-by: Kamen Mazdrashki <kamenim at samba.org>
> > > > ---
> > > >  python/samba/tests/__init__.py | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/python/samba/tests/__init__.py
> > > b/python/samba/tests/__init__.py
> > > > index 465a8f1..db09215 100644
> > > > --- a/python/samba/tests/__init__.py
> > > > +++ b/python/samba/tests/__init__.py
> > > > @@ -92,7 +92,7 @@ def env_loadparm():
> > > >      try:
> > > >          lp.load(os.environ["SMB_CONF_PATH"])
> > > >      except KeyError:
> > > > -        raise Exception("SMB_CONF_PATH not set")
> > > > +        raise KeyError("SMB_CONF_PATH not set")
> > > >      return lp
> > > >
> > > >
> > > > --
> > > > 1.9.1
> > > Reviewed-By: Jelmer Vernooij <jelmer at samba.org>
> > >
> > > > From 8758d8a2f383f70341a1f059a3944654c8d22339 Mon Sep 17 00:00:00
> 2001
> > > > From: Kamen Mazdrashki <kamenim at samba.org>
> > > > Date: Tue, 2 Dec 2014 05:18:39 +0100
> > > > Subject: [PATCH 2/2] s4-tests/get_loadparm: Extend the helper to load
> > > default
> > > >  params if requested
> > > >
> > > > Change-Id: I7df1c29e573fe9ed24069efc28b94c5fc8c18aa1
> > > > Signed-off-by: Kamen Mazdrashki <kamenim at samba.org>
> > > > ---
> > > >  python/samba/tests/__init__.py | 11 +++++++++--
> > > >  1 file changed, 9 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/python/samba/tests/__init__.py
> > > b/python/samba/tests/__init__.py
> > > > index db09215..76fe095 100644
> > > > --- a/python/samba/tests/__init__.py
> > > > +++ b/python/samba/tests/__init__.py
> > > > @@ -50,8 +50,15 @@ class TestCase(TesttoolsTestCase):
> > > >              samba.set_debug_level(test_debug_level)
> > > >              self.addCleanup(samba.set_debug_level, test_debug_level)
> > > >
> > > > -    def get_loadparm(self):
> > > > -        return env_loadparm()
> > > > +    def get_loadparm(self, load_default=False):
> > > > +        try:
> > > > +            return env_loadparm()
> > > > +        except KeyError:
> > > > +            if not load_default:
> > > > +                raise
> > > > +        lp = param.LoadParm()
> > > > +        lp.load_default()
> > > > +        return lp
> > > >
> > > >      def get_credentials(self):
> > > >          return cmdline_credentials
> > > What is the reason for this change?
> > >
> >
> > It was primary  for my convenience - not to make a dummi smb.conf +
> > defining another env variable.
> > But yeah, your point is quite fair.
> > I am using it like this:
> >
> >
> https://github.com/kamenim/samba/commit/75a87863e8be5d23e28e79cab4e3c0d829ba24ee
> > So the test author explicitly has to say "I don't car that much about
> > default configuration"
> > Especially in the above case, where "samba.tests.connect_samdb_env" is
> > going to
> > populate username/password anyway.
> > So in the end, test writter get "i do care" behaviour out of the box.
> > But I get you point and if you NAK it is ok (i will just update the
> comment
> > in the begging of the test
> > to remind myself how to run it from command line :)
> I would prefer to require explicit configuration to prevent test isolation
> issues.
>
> If the test doesn't really need a configuration, then it shouldn't be
> calling
> get_loadparm() but rather just calling param.LoadParm() without
> load_default().
>

Tried this and works for me


>
> What specific test are you looking at?
>

I am looking at my test :). See:
https://github.com/kamenim/samba/commit/75a87863e8be5d23e28e79cab4e3c0d829ba24ee

But anyway, please discard this commit. I wll remove it

Cheers,
Kamen


>
> Cheers,
>
> Jelmer
>