On Oct. 30, 2013, 2:44 p.m., Radek Novacek wrote:

...

src/software/test/testSoftware.py, line 98 http://reviewboard-openlmi.rhcloud.com/r/1147/diff/1/?file=6187#file6187line98

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>.

Yeah, some kind of description "why?" would be helpful. I would use regular expression: string = re.sub(r'-\d+:', '-', string)

On Oct. 30, 2013, 2:44 p.m., Radek Novacek wrote:

...

src/software/test/testSoftware.py, line 255 http://reviewboard-openlmi.rhcloud.com/r/1147/diff/1/?file=6187#file6187line255

use "assertListEqual", it produces better output when the test fails

Unfortunately this is available since python 2.7.

On Oct. 30, 2013, 2:44 p.m., Radek Novacek wrote:

...

src/software/test/testSoftware.py, line 592 http://reviewboard-openlmi.rhcloud.com/r/1147/diff/1/?file=6187#file6187line592

self.assertIsNone()

Again, since python 2.7.

On Oct. 30, 2013, 2:44 p.m., Radek Novacek wrote:

...

src/software/test/testSoftware.py, line 312 http://reviewboard-openlmi.rhcloud.com/r/1147/diff/1/?file=6187#file6187line312

You can use

.InstalledSoftware.InstanceID

instead of

.path["InstalledSoftware"]["InstanceID"]

From what I know:

.InstalledSoftware["InstanceID"] Because InstalledSoftware is an instance of pywbem.CIMInstanceName, which doesn't allow for smart attribute access.

On Oct. 30, 2013, 2:44 p.m., Radek Novacek wrote:

...

src/software/test/testSoftware.py, line 300 http://reviewboard-openlmi.rhcloud.com/r/1147/diff/1/?file=6187#file6187line300

assertEqual -> assertListEqual

Since python 2.7.

On Oct. 30, 2013, 2:44 p.m., Radek Novacek wrote:

...

src/software/test/testSoftware.py, line 284 http://reviewboard-openlmi.rhcloud.com/r/1147/diff/1/?file=6187#file6187line284

The matching against 'yum list available' is missing. If it is on purpose, add at least a TODO comment.

This should be achievable. Try to remove installed packages and duplicities from lmi list and compare them. (I haven't tried).

- Michal

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

On Oct. 30, 2013, 2:03 p.m., Jan Grec wrote:

---

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

---

(Updated Oct. 30, 2013, 2:03 p.m.)

Review request for OpenLMI Developers.

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

Diff: http://reviewboard-openlmi.rhcloud.com/r/1147/diff/

Testing

Thanks,

Jan Grec

On Oct. 30, 2013, 3:44 p.m., Radek Novacek wrote:

...

src/software/test/testSoftware.py, line 284 http://reviewboard-openlmi.rhcloud.com/r/1147/diff/1/?file=6187#file6187line284

The matching against 'yum list available' is missing. If it is on purpose, add at least a TODO comment.

Michal Minar wrote: This should be achievable. Try to remove installed packages and duplicities from lmi list and compare them. (I haven't tried).

Skipping this one for now.

Although it'd be great to know exactly whether listing available packages returns correct available packages, there's no exact process to get them in lmi itself.

From packages returned, we'd have to remove all packages with package names that are already installed and their epoch or version is lesser omitting updates which requires package name parsing etc. etc.

I suppose this is going to be an openlmi-scripts functionality and therefore should be tested there, not in lmi itself.

- Jan

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

On Oct. 30, 2013, 3:03 p.m., Jan Grec wrote:

---

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

---

(Updated Oct. 30, 2013, 3:03 p.m.)

Review request for OpenLMI Developers.

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

Diff: http://reviewboard-openlmi.rhcloud.com/r/1147/diff/

Testing

Thanks,

Jan Grec

On Oct. 30, 2013, 3:44 p.m., Radek Novacek wrote:

...

src/software/test/testSoftware.py, line 255 http://reviewboard-openlmi.rhcloud.com/r/1147/diff/1/?file=6187#file6187line255

use "assertListEqual", it produces better output when the test fails

Michal Minar wrote: Unfortunately this is available since python 2.7.

Jan Grec wrote: I'm afraid there are more pieces of tests code that won't work on 2.6

Shall we strictly support python 2.6 and more, or is it ok to run with 2.7 and eventually rewrite tests for older python versions when it's needed?

Michal Minar wrote: I'd rather stick with code compatible with 2.6. True, not all of the current code works with 2.6 but this is not the reason to use new features from 2.7 since we know these tests need to pass on 2.6 sooner or later.

Okay, I'll change it back to assertEqual.

Also docs for assertEqual says: "In addition, if first and second are the exact same type and one of list, tuple, dict, set, frozenset or unicode or any type that a subclass registers with addTypeEqualityFunc() the type-specific equality function will be called in order to generate a more useful default error message (see also the list of type-specific methods)." so I guess assertEqual doesn't matter.

- Jan

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

On Oct. 30, 2013, 3:03 p.m., Jan Grec wrote:

---

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

---

(Updated Oct. 30, 2013, 3:03 p.m.)

Review request for OpenLMI Developers.

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

Diff: http://reviewboard-openlmi.rhcloud.com/r/1147/diff/

Testing

Thanks,

Jan Grec