----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-openlmi.rhcloud.com/r/847/ -----------------------------------------------------------
Review request for OpenLMI Developers.
Repository: openlmi-providers
Description -------
journald: Add iterator tests
Tests basic iterator operations, assumes accessible journal and several (> 10) records available.
Also comes with a simple example script for downloading complete journal.
-- This is the largest chunk of Python code I've ever written! \o/
Diffs -----
src/journald/test/TestIterators.py PRE-CREATION src/journald/test/list-journal PRE-CREATION
Diff: http://reviewboard-openlmi.rhcloud.com/r/847/diff/
Testing -------
Thanks,
Tomáš Bžatek
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-openlmi.rhcloud.com/r/847/#review1154 -----------------------------------------------------------
src/journald/test/TestIterators.py http://reviewboard-openlmi.rhcloud.com/r/847/#comment686
IMHO this is more readable: self.assertEqual(retval, 0)
src/journald/test/TestIterators.py http://reviewboard-openlmi.rhcloud.com/r/847/#comment685
Wow, I wonder whether this id is unique enough... :-D IPv6 address sucks!
src/journald/test/TestIterators.py http://reviewboard-openlmi.rhcloud.com/r/847/#comment687
unittest has means even for exception handling:
with self.assertRaises(pywbem.CIMError) as cm: func(*args, **kwds) self.assertEqual(pywbem.CIM_ERR_INVALID_PARAMETER, cm.exception.args[0])
And since this is a common use case in your test, you may save a lot by defining this method: def assertRaisesCIM(self, cim_err_code, func, *args, **kwds): """ This test passes if given function called with supplied arguements raises pywbem.CIMError with given cim error code. """ with self.assertRaises(pywbem.CIMError) as cm: func(*args, **kwds) self.assertEqual(cim_err_code, cm.exception.args[0]) resulting in: self.assertRaisesCIM(pywbem.CIM_ERR_INVALID_PARAMETER, self.wbemconnection.InvokeMethod, MethodName="CancelIteration", ObjectName=inst.path, IterationIdentifier=rfirst['IterationIdentifier'])
src/journald/test/list-journal http://reviewboard-openlmi.rhcloud.com/r/847/#comment688
I know it's a bit more code, but makes it usable elsewhere: url = os.environ.get('LMI_CIMOM_URL', 'localhost') username = os.environ.get('LMI_CIMOM_USERNAME', "pegasus") password = os.environ.get('LMI_CIMOM_PASSWORD', "heslo") c = connect(url, username, password)
- Michal Minar
On Sept. 10, 2013, 2:32 p.m., Tomáš Bžatek wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-openlmi.rhcloud.com/r/847/
(Updated Sept. 10, 2013, 2:32 p.m.)
Review request for OpenLMI Developers.
Repository: openlmi-providers
Description
journald: Add iterator tests
Tests basic iterator operations, assumes accessible journal and several (> 10) records available. Also comes with a simple example script for downloading complete journal.
-- This is the largest chunk of Python code I've ever written! \o/
Diffs
src/journald/test/TestIterators.py PRE-CREATION src/journald/test/list-journal PRE-CREATION
Diff: http://reviewboard-openlmi.rhcloud.com/r/847/diff/
Testing
Thanks,
Tomáš Bžatek
On Sept. 11, 2013, 6:39 a.m., Michal Minar wrote:
And if you'd stick to 80 chars per line, you could even double the lines of code :-).
- Michal
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-openlmi.rhcloud.com/r/847/#review1154 -----------------------------------------------------------
On Sept. 10, 2013, 2:32 p.m., Tomáš Bžatek wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-openlmi.rhcloud.com/r/847/
(Updated Sept. 10, 2013, 2:32 p.m.)
Review request for OpenLMI Developers.
Repository: openlmi-providers
Description
journald: Add iterator tests
Tests basic iterator operations, assumes accessible journal and several (> 10) records available. Also comes with a simple example script for downloading complete journal.
-- This is the largest chunk of Python code I've ever written! \o/
Diffs
src/journald/test/TestIterators.py PRE-CREATION src/journald/test/list-journal PRE-CREATION
Diff: http://reviewboard-openlmi.rhcloud.com/r/847/diff/
Testing
Thanks,
Tomáš Bžatek
On Sept. 11, 2013, 6:39 a.m., Michal Minar wrote:
Michal Minar wrote: And if you'd stick to 80 chars per line, you could even double the lines of code :-).
I'm not a fan of 80 chars per line especially when python code is so strict with indenting. Reformatted some lines though. I'm not paid by SLOC btw. ;-)
On Sept. 11, 2013, 6:39 a.m., Michal Minar wrote:
src/journald/test/list-journal, line 3 http://reviewboard-openlmi.rhcloud.com/r/847/diff/1/?file=4651#file4651line3
I know it's a bit more code, but makes it usable elsewhere: url = os.environ.get('LMI_CIMOM_URL', 'localhost') username = os.environ.get('LMI_CIMOM_USERNAME', "pegasus") password = os.environ.get('LMI_CIMOM_PASSWORD', "heslo") c = connect(url, username, password)
To be honest, I'm not even sure this belongs to openlmi-providers since it requires lmishell to run. It was meant as a very simple example, with planned publishing on the docs page/wiki.
On Sept. 11, 2013, 6:39 a.m., Michal Minar wrote:
src/journald/test/TestIterators.py, line 64 http://reviewboard-openlmi.rhcloud.com/r/847/diff/1/?file=4650#file4650line64
Wow, I wonder whether this id is unique enough... :-D IPv6 address sucks!
The third (largest) part is a standard journald cursor string, unique enough :-) Each part of the ID has its own role, allowing us to do nice things like session recovery.
On Sept. 11, 2013, 6:39 a.m., Michal Minar wrote:
src/journald/test/TestIterators.py, lines 65-69 http://reviewboard-openlmi.rhcloud.com/r/847/diff/1/?file=4650#file4650line65
unittest has means even for exception handling: with self.assertRaises(pywbem.CIMError) as cm: func(*args, **kwds) self.assertEqual(pywbem.CIM_ERR_INVALID_PARAMETER, cm.exception.args[0]) And since this is a common use case in your test, you may save a lot by defining this method: def assertRaisesCIM(self, cim_err_code, func, *args, **kwds): """ This test passes if given function called with supplied arguements raises pywbem.CIMError with given cim error code. """ with self.assertRaises(pywbem.CIMError) as cm: func(*args, **kwds) self.assertEqual(cim_err_code, cm.exception.args[0]) resulting in: self.assertRaisesCIM(pywbem.CIM_ERR_INVALID_PARAMETER, self.wbemconnection.InvokeMethod, MethodName="CancelIteration", ObjectName=inst.path, IterationIdentifier=rfirst['IterationIdentifier'])
Oh nice, I was not aware of such option. Thanks for pointing this out, at least I learned something new.
- Tomáš
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-openlmi.rhcloud.com/r/847/#review1154 -----------------------------------------------------------
On Sept. 10, 2013, 2:32 p.m., Tomáš Bžatek wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-openlmi.rhcloud.com/r/847/
(Updated Sept. 10, 2013, 2:32 p.m.)
Review request for OpenLMI Developers.
Repository: openlmi-providers
Description
journald: Add iterator tests
Tests basic iterator operations, assumes accessible journal and several (> 10) records available. Also comes with a simple example script for downloading complete journal.
-- This is the largest chunk of Python code I've ever written! \o/
Diffs
src/journald/test/TestIterators.py PRE-CREATION src/journald/test/list-journal PRE-CREATION
Diff: http://reviewboard-openlmi.rhcloud.com/r/847/diff/
Testing
Thanks,
Tomáš Bžatek
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-openlmi.rhcloud.com/r/847/ -----------------------------------------------------------
(Updated Sept. 11, 2013, 2:24 p.m.)
Review request for OpenLMI Developers.
Changes -------
Updated patch with all the issues implemented.
Repository: openlmi-providers
Description -------
journald: Add iterator tests
Tests basic iterator operations, assumes accessible journal and several (> 10) records available.
Also comes with a simple example script for downloading complete journal.
-- This is the largest chunk of Python code I've ever written! \o/
Diffs (updated) -----
src/journald/test/list-journal PRE-CREATION src/journald/test/TestIterators.py PRE-CREATION src/journald/test/common.py PRE-CREATION
Diff: http://reviewboard-openlmi.rhcloud.com/r/847/diff/
Testing -------
Thanks,
Tomáš Bžatek
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-openlmi.rhcloud.com/r/847/#review1159 -----------------------------------------------------------
Ship it!
Ship It!
- Michal Minar
On Sept. 11, 2013, 2:24 p.m., Tomáš Bžatek wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-openlmi.rhcloud.com/r/847/
(Updated Sept. 11, 2013, 2:24 p.m.)
Review request for OpenLMI Developers.
Repository: openlmi-providers
Description
journald: Add iterator tests
Tests basic iterator operations, assumes accessible journal and several (> 10) records available. Also comes with a simple example script for downloading complete journal.
-- This is the largest chunk of Python code I've ever written! \o/
Diffs
src/journald/test/list-journal PRE-CREATION src/journald/test/TestIterators.py PRE-CREATION src/journald/test/common.py PRE-CREATION
Diff: http://reviewboard-openlmi.rhcloud.com/r/847/diff/
Testing
Thanks,
Tomáš Bžatek
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-openlmi.rhcloud.com/r/847/ -----------------------------------------------------------
(Updated Sept. 30, 2013, 12:16 p.m.)
Review request for OpenLMI Developers.
Changes -------
Posted updated patch to reflect changes in review #837. Re-review probably not needed.
Repository: openlmi-providers
Description -------
journald: Add iterator tests
Tests basic iterator operations, assumes accessible journal and several (> 10) records available.
Also comes with a simple example script for downloading complete journal.
-- This is the largest chunk of Python code I've ever written! \o/
Diffs (updated) -----
src/journald/test/TestIterators.py PRE-CREATION src/journald/test/common.py d65b5d1646e8ee10ffda1aa437622cf95484522b src/journald/test/list-journal PRE-CREATION
Diff: http://reviewboard-openlmi.rhcloud.com/r/847/diff/
Testing -------
Thanks,
Tomáš Bžatek
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-openlmi.rhcloud.com/r/847/ -----------------------------------------------------------
(Updated Oct. 8, 2013, 10:10 a.m.)
Status ------
This change has been marked as submitted.
Review request for OpenLMI Developers.
Repository: openlmi-providers
Description -------
journald: Add iterator tests
Tests basic iterator operations, assumes accessible journal and several (> 10) records available.
Also comes with a simple example script for downloading complete journal.
-- This is the largest chunk of Python code I've ever written! \o/
Diffs -----
src/journald/test/TestIterators.py PRE-CREATION src/journald/test/common.py d65b5d1646e8ee10ffda1aa437622cf95484522b src/journald/test/list-journal PRE-CREATION
Diff: http://reviewboard-openlmi.rhcloud.com/r/847/diff/
Testing -------
Thanks,
Tomáš Bžatek
openlmi-reviews@lists.fedorahosted.org