This is an automatically generated e-mail. To reply, visit: http://reviewboard-openlmi.rhcloud.com/r/1147/

src/software/test/testSoftware.py (Diff revision 1)
48
    def setUpClass(self):
Use 'cls' instead of 'self' in classmethods. See pep8: http://www.python.org/dev/peps/pep-0008/#function-and-method-arguments

src/software/test/testSoftware.py (Diff revision 1)
98
def _parse_rpm_installed_package_name(string):
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>.

src/software/test/testSoftware.py (Diff revision 1)
181
        if (word[-6:] in ["x86_64", "noarch"]) or \
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.

src/software/test/testSoftware.py (Diff revision 1)
255
        self.assertEqual(sorted(package_list), sorted(rpm_packages))
use "assertListEqual", it produces better output when the test fails

src/software/test/testSoftware.py (Diff revision 1)
284
The matching against 'yum list available' is missing. If it is on purpose, add at least a TODO comment.

src/software/test/testSoftware.py (Diff revision 1)
300
        self.assertEqual(repos, yum_repos)
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"]

src/software/test/testSoftware.py (Diff revision 1)
475
            OpenLMI enables enabled repository - doesn't rise an error
typo: rise -> raise (occurs multiple times)

src/software/test/testSoftware.py (Diff revision 1)
566
        cs = self.cim.PG_ComputerSystem.first_instance_name()
Don't use PG_ComputerSystem directly, there should be LMI_CS_CLASSNAME env var, see https://fedorahosted.org/openlmi/ticket/161

src/software/test/testSoftware.py (Diff revision 1)
592
        self.assertTrue(isinstance(installed_assoc, None.__class__))
self.assertIsNone()

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