Review Request 710: First upstream test for OpenLMI-Service provider.

[Stephen Gallagher]

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

Review request for OpenLMI Developers.


Description
-------

First upstream test for OpenLMI-Service provider.


Diffs
-----


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


Testing
-------

Now.


File Attachments
----------------

First upstream test for service provider
 
http://reviewboard-openlmi.rhcloud.com/static/media/uploaded/files/2013/0...


Thanks,

Robin Hack

[Stephen Gallagher]

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

(Updated Aug. 15, 2013, 8:59 a.m.)


Review request for OpenLMI Developers.


Changes
-------

Old version of test has been removed.


Description
-------

First upstream test for OpenLMI-Service provider.


Diffs
-----


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


Testing
-------

Now.


File Attachments (updated)
----------------

Service provider test
 
http://reviewboard-openlmi.rhcloud.com/static/media/uploaded/files/2013/0...


Thanks,

Robin Hack

[Stephen Gallagher]

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


Please post the review as proper diff to current git HEAD next time, it's hard to
review it as attachment.

Overall, the tests are fine, just some polishing is needed.

Line 27:
USER = os.environ["LMI_CIMOM_USERNAME"] if
os.environ.has_key("LMI_CIMOM_USERNAME")     else "root"

USER = os.environ.get("LMI_CIMOM_USERNAME", "root") looks so much
nicer


Line 33:
class TestConnection(unittest.TestCase):

IMHO this class is useless and should be removed.
On the other way, it might be nice if you have a base class with setUp(), which sets up
lmishell connection etc., and each test case inherits it. Probably in separate file.


Line 41:
class TestProviders(unittest.TestCase):

IMHO better name (TestService?) should be chosen.


Line 51:
    def test_list_services(self):

Please write some documentation for the test. For example:

    def test_list_services(self):
        """
        Check all services have LMI_Service instance with correct status.

        Compare all LMI_Services with output of
            systemcl list-unit-files -t service
        """
    
The first line should be short test name (or slogan), it's shown when nosetests -v is
run, other lines are up to you.


Line 48:
ret = subprocess.call(cmd.split ());

No space between 'split' and '()' please.


- Jan Safranek


On Aug. 15, 2013, 10:59 a.m., Robin Hack wrote:
>  
>  -----------------------------------------------------------
>  This is an automatically generated e-mail. To reply, visit:
>  http://reviewboard-openlmi.rhcloud.com/r/710/
>  -----------------------------------------------------------
>  
>  (Updated Aug. 15, 2013, 10:59 a.m.)
>  
>  
>  Review request for OpenLMI Developers.
>  
>  
>  Description
>  -------
>  
>  First upstream test for OpenLMI-Service provider.
>  
>  
>  Diffs
>  -----
>  
>  
>  Diff: http://reviewboard-openlmi.rhcloud.com/r/710/diff/
>  
>  
>  Testing
>  -------
>  
>  Now.
>  
>  
>  File Attachments
>  ----------------
>  
>  Service provider test
>   
> http://reviewboard-openlmi.rhcloud.com/static/media/uploaded/files/2013/0...
>  
>  
>  Thanks,
>  
>  Robin Hack
>  
>

[Stephen Gallagher]

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

(Updated Nov. 25, 2013, 1:25 p.m.)


Status
------

This change has been discarded.


Review request for OpenLMI Developers.


Description
-------

First upstream test for OpenLMI-Service provider.


Diffs
-----


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


Testing
-------

Now.


File Attachments
----------------

Service provider test
 
http://reviewboard-openlmi.rhcloud.com/static/media/uploaded/files/2013/0...


Thanks,

Robin Hack