Use 'cls' instead of 'self' in classmethods. See pep8: http://www.python.org/dev/peps/pep-0008/#function-and-method-arguments
What is the purpose of this function? Remove epoch from NVR? If so then note that epoch could be arbitrary number, this works only for epoch <0..9>.
I would rather use endswith:
for arch in ("x86_64", "noarch", "amd64", "i686", "i386"):
if word.endswith(arch):
...
But that's not really an issue.
use "assertListEqual", it produces better output when the test fails
The matching against 'yum list available' is missing. If it is on purpose, add at least a TODO comment.
assertEqual -> assertListEqual
src/software/test/testSoftware.py
(Diff revision 1)
|
|
|
312 |
package_name = self.cim.LMI_InstalledSoftwareIdentity.first_instance().path["InstalledSoftware"]["InstanceID"] |
You can use
.InstalledSoftware.InstanceID
instead of
.path["InstalledSoftware"]["InstanceID"]
typo: rise -> raise (occurs multiple times)
Don't use PG_ComputerSystem directly, there should be LMI_CS_CLASSNAME env var, see https://fedorahosted.org/openlmi/ticket/161
The test suite looks good to me in general. I don't have any objections against merging it into upstream repo, but I would like to let Michal decide if he is satisfied with the test suite this way.
- Radek Novacek
On October 30th, 2013, 3:03 p.m. CET, Jan Grec wrote:
Review request for OpenLMI Developers.
By Jan Grec.
Updated Oct. 30, 2013, 3:03 p.m.
Repository:
openlmi-providers
Description
software - tests: software test suite with basic tests
This is only a "current-status" review. I'll be very thankful for any comments on what's wrong, how the structure should look like, etc.
My TODO is to
* repair all broken tests
* add all tests mentioned
* bind test suite with new LmiTestCase class
I apologize for inconvenience, but I wasn't able to save previous review notes.
|
Diffs
- src/software/test/lmi-test.repo (PRE-CREATION)
- src/software/test/testSoftware.py (PRE-CREATION)
View Diff
|