virajjasani commented on a change in pull request #2454:
URL: https://github.com/apache/hbase/pull/2454#discussion_r500845391



##########
File path: 
hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterChoreScheduled.java
##########
@@ -114,22 +113,27 @@ public void testDefaultScheduledChores() throws Exception 
{
     hbckChoreTestChoreField.testIfChoreScheduled(hbckChore);
   }
 
-
+  /**
+   * Reflect into the {@link HMaster} instance and find by field name a 
specified instance
+   * of {@link ScheduledChore}.
+   */
   private static class TestChoreField<E extends ScheduledChore> {
 
-    private E getChoreObj(String fieldName) throws NoSuchFieldException,
-        IllegalAccessException {
-      Field masterField = HMaster.class.getDeclaredField(fieldName);
-      masterField.setAccessible(true);
-      E choreFieldVal = (E) masterField.get(hMaster);
-      return choreFieldVal;
+    @SuppressWarnings("unchecked")
+    private E getChoreObj(String fieldName) {
+      try {
+        Field masterField = HMaster.class.getDeclaredField(fieldName);
+        masterField.setAccessible(true);
+        return (E) masterField.get(hMaster);
+      } catch (Exception e) {
+        throw new AssertionError(
+          "Unable to retrieve field '" + fieldName + "' from HMaster 
instance.", e);

Review comment:
       Given that this test is fragile in the way we use reflection, having 
this nice message in error clearly points out what the issue is and dev can 
just focus on refactor part to ensure chore object that is being refactored is 
somehow covered in this test. Nice error message 👍 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to