[PATCH] create python test for samba-tool time command
sdague at linux.vnet.ibm.com
Mon Oct 24 08:31:12 MDT 2011
On 10/23/2011 06:12 PM, Jelmer Vernooij wrote:
> This file lacks a copyright header - does the source test pass for you?
Source test? Is there something I should be running here?
>> +# # domain stuff
>> +# "DOMAIN",
>> +# "REALM",
>> +# # domain controller stuff
>> +# "DC_SERVER",
>> +# "DC_SERVER_IP",
>> +# "DC_NETBIOSNAME",
>> +# "DC_NETBIOSALIAS",
>> +# # domain member
>> +# "MEMBER_SERVER",
>> +# "MEMBER_SERVER_IP",
>> +# "MEMBER_NETBIOSNAME",
>> +# "MEMBER_NETBIOSALIAS",
>> +# # rpc proxy controller stuff
>> +# "RPC_PROXY_SERVER",
>> +# "RPC_PROXY_SERVER_IP",
>> +# "RPC_PROXY_NETBIOSNAME",
>> +# "RPC_PROXY_NETBIOSALIAS",
>> +# # domain controller stuff for Vampired DC
>> +# "VAMPIRE_DC_SERVER",
>> +# "VAMPIRE_DC_SERVER_IP",
>> +# "VAMPIRE_DC_NETBIOSNAME",
>> +# "VAMPIRE_DC_NETBIOSALIAS",
>> +# # server stuff
>> +# "SERVER",
>> +# "SERVER_IP",
>> +# "NETBIOSNAME",
>> +# "NETBIOSALIAS",
>> +# # user stuff
>> +# "USERNAME",
>> +# "USERID",
>> +# "PASSWORD",
>> +# "DC_USERNAME",
>> +# "DC_PASSWORD",
>> +# # misc stuff
>> +# "KRB5_CONFIG",
>> +# "WINBINDD_SOCKET_DIR",
>> +# "WINBINDD_PRIV_PIPE_DIR",
>> +# "NMBD_SOCKET_DIR",
>> +# "LOCAL_PATH"
>> +# These can all be accesses via os.environ["VARIBLENAME"] when needed
> Rather than duplicating this list here, it would probably be better to
> point at the master list of the variables. The risk of duplicating it is
> that it will get out of date when the main list gets changed.
Is there a master list somewhere? I just went and found it in
selftest.pl, and it seemed confusing to tell folks to read a perl file
when writing a python test, especially if they weren't heavily
experienced with both.
>> +import os
>> +import sys
>> +from cStringIO import StringIO
>> +from samba.netcmd.main import cmd_sambatool
>> +import samba.tests
>> +class SambaToolCmdTest(samba.tests.TestCase):
>> + def runcmd(self, name, *args):
>> + cmd = cmd_sambatool.subcommands[name]
>> + cmd.outf = StringIO()
>> + cmd.errf = StringIO()
>> + result = cmd._run(name, *args)
>> + return (result, cmd.outf.getvalue(), cmd.errf.getvalue())
> This duplicates code from samba.tests.netcmd - it would be nicer to
> derive from the class there instead.
There are some similarities, but the calling interface is different
enough that I think for now it warants being on it's own. That becomes
especially true when dealing with commands with subcommands, as you
actually need to reach down another layer otherwise the StringIO
redirection doesn't work. I found that out the hard way when working on
Also, calling by class might make sense for the iteration commands, but
when trying to write tests for commands, doing calling those by string
(i.e. self.runsubcmd("user", "add", .....) ends up being a bit more clear.
If you really want to make them based off the same base class, I think
we should move netcmd into samba_tool directory.
>> + def assertCmdSuccess(self, val, msg=""):
>> + self.assertIsNone(val, msg)
>> + def assertCmdFail(self, val, msg=""):
>> + self.assertIsNotNone(val, msg)
> These methods seem pretty useless given output and stderr are never
> going to be None, they'll always be a string.
result is currently None if success, and a number (usually -1) if fail,
though I'm not sure that will always be the case. I added them in mostly
to make it clear in the test what was being tested for.
>> diff --git a/source4/scripting/python/samba/tests/samba_tool/timecmd.py
>> new file mode 100644
>> index 0000000..2de9685
>> --- /dev/null
>> +++ b/source4/scripting/python/samba/tests/samba_tool/timecmd.py
>> +class TimeCmdTestCase(SambaToolCmdTest):
>> + """Tests for samba-tool time subcommands"""
>> + def test_timeget(self):
>> + """Run time against the server and make sure it looks accurate"""
>> + (result, out, err) = self.runcmd("time", os.environ["SERVER"])
>> + self.assertCmdSuccess(result, "Ensuring time ran successfully")
>> + timefmt = strptime(out, "%a %b %d %H:%M:%S %Y %Z\n")
>> + servertime = int(mktime(timefmt))
>> + now = int(mktime(localtime()))
>> + # because there is a race here, allow up to 5 seconds difference in times
>> + delta = 5
>> + self.assertTrue((servertime> (now - delta) and (servertime< (now +
> delta)), "Time is now"))
>> + def test_timefail(self):
>> + """Run time against a non-existant server, and make sure it fails"""
>> + try:
>> + (result, out, err) = self.runcmd("time", "notaserver")
>> + self.assertCmdFail(result, "Ensuring time failed as expected")
>> + except:
>> + self.assertTrue(True, "Failure caused an exception, ok and expected
> for now")
> except: (rather than e.g. "except Exception:") is evil (it can "eat"
> exceptions like KeyboardInterrupt). Rather than using this, it's nicer
> and easier to use self.assertRaises, e.g.
> self.assertRaises(self.runcmd, "time", "notaserver")
Agreed. I will update with that.
Do you want me to respin what's here? Or is this going to be applied and
we'll build off of it?
IBM Linux Technology Center
email: sldague at us.ibm.com
alt-email: sdague at linux.vnet.ibm.com
More information about the samba-technical