[GitHub] cloudstack pull request: Notify listeners when a host has been add...

[mike-tutkowski <gi...@git.apache.org>]

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