cloudstack-pull-rats #654 SUCCESS
This pull request looks good

cloudstack-pull-analysis #603 SUCCESS
This pull request looks good

LGTM

@DaanHoogland Why log when an exception is thrown? In a test when an exception is thrown it should either fail, or when @test(expected = --exception.class--) is defined succeed.

Right?

@borisroman the expeted attribute is not there and as I understand with reason. The exceptions are part of the contract and need to be handled by the client. The test needs to verify the result irrespectively. @koushik-das Do I formulate this correct?

@DaanHoogland @borisroman The test needs to invoke a protected method from a class and so is done using reflection. The test case already asserts for the method not being null. Now reflection methods throws some standard exceptions which either needs to be handled or thrown as per Java method contract. Since the null check is already there, the exceptions can be safely ignored.

LGTM

Do you guys really think that is a good idea to log exceptions in a test case?
If the case is just to ignore them, it could be done without logging.

the reason for ignoring should be made more explicit in one way or the other. I'm not hellbound on logging but I see no problem in it. A caught and ignored exception without any action is a bad code snippit I want to be dealt with, even in tests.

If an exception occurs, is the test still going to pass?
If that is not the case, I would use "throws Exception" on each one of those tests.

not "throws Exception" but a more specific throws. In this case the exceptions are silently ignored as part of a happy flow. I don't like that but I'm not going to waste much more time on it

Sure, I think that a more specific “throws” clause has to be used. Throws exception, was just an expression.