DaanHoogland commented on a change in pull request #2776: Issue 2774: Changed
the implementation of isVolumeOnManagedStorage(VolumeInfo) to…
URL: https://github.com/apache/cloudstack/pull/2776#discussion_r207131571
##########
File path:
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
##########
@@ -196,10 +196,16 @@ public StrategyPriority canHandle(DataObject srcData,
DataObject destData) {
}
private boolean isVolumeOnManagedStorage(VolumeInfo volumeInfo) {
Review comment:
ok, i am torn on this;
yes methods should be small fit easily in one screen and be easy to read.
yes every bit (or, if we are forgiving every line) of code should be unit
tested.
testing private methods however required trickery that I don't like.
I created a test to test for a particular behaviour of canHandle() that
comes down to isVolumeOnManageStorage() (@mike-tutkowski can you merge it?) in
this case because I felt it was better then to test a trivial private method.
Do we expect that not even the most trivial methods are tested implicitly but
instead stubbed? We could draw a strict line but I'm for case by case
evaluation. Code coverage in tests is more important to me.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
