cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mike-tutkowski <...@git.apache.org>
Subject [GitHub] cloudstack pull request: Notify listeners when a host has been add...
Date Thu, 01 Oct 2015 04:57:58 GMT
Github user mike-tutkowski commented on the pull request:

    https://github.com/apache/cloudstack/pull/816#issuecomment-144619954
  
    So, I've been paging through the diff files to see what might be a good candidate for
unit testing.
    
    Unfortunately, I don't really see much.
    
    What I do see (in terms of added/changed code) is the following:
    
    * Removal of a constructor (compiler would catch any issues here).
    * Renaming of variables (typically adding a "_" before a member variable).
    * Some new setters and getters (technically could do unit testing here to confirm that
what we place in the object is what we get out of it, but most of the time we are just directly
setting a member variable and retrieving from it, so it's pretty low risk).
    * Adding "private" and "protected" where previously we had "public" (the compiler should
catch issues here).
    * Making a previously private method public and making it accessible via an interface.
    * Adding three methods to the HypervisorHostListener interface: hostAdded, hostAboutToBeRemoved,
and hostRemoved (but not really having any hooks into the system to be able to "pretend" like
I'm adding or removing a host).
    * Removing the word "public" from the methods of an interface since all methods in an
interface are public in Java.
    * Adding three methods to the Listener interface: processHostAdded, processHostAboutToBeRemoved,
and processHostRemoved (but not really having any hooks into the system to be able to "pretend"
like I'm adding or removing a host). Also, this leads to a need to make a simple, default
"do nothing" implementation of these methods for about 30 classes.
    * Adding an extra command to allow while a host is in maintenance mode (but not having
any way to simulate maintenance mode via unit testing).
    * Adding a new command that VmwareResource can respond to. However, the command requires
interaction with an ESXi host, so can't really be tested in isolation (i.e. via unit testing).
    * Changes to the way DB locking is performed in a couple scenarios with the SolidFire
storage plug-ins (requiring an underlying DB).
    * Reformatting of text to better conform to ACS formatting guidelines.
    * Removal of "dead" code that I came across while working on this ticket.
    
    I'd be happy to entertain ideas on unit testing this PR, but I'm thinking we'd get more
"mileage" here out of adding integration tests.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message