[GitHub] [accumulo] cshannon commented on a diff in pull request #3106: Remove deprecated properties

[GitBox <gi...@apache.org>]

cshannon commented on code in PR #3106:
URL: https://github.com/apache/accumulo/pull/3106#discussion_r1038870819


##########
core/src/test/java/org/apache/accumulo/core/conf/AccumuloConfigurationTest.java:
##########
@@ -394,42 +386,17 @@ public void testScanExecutors() {
 
   // note: this is hard to test if there aren't any deprecated properties
   // if that's the case, just comment this test out or create a dummy deprecated property
-  @SuppressWarnings("deprecation")
   @Test
   public void testResolveDeprecated() {
     var conf = new ConfigurationCopy();
 
-    // deprecated first argument
-    var e1 = assertThrows(IllegalArgumentException.class, () -> conf
-        .resolve(Property.INSTANCE_DFS_DIR, Property.INSTANCE_DFS_URI, Property.INSTANCE_DFS_URI));
-    assertEquals("Unexpected deprecated INSTANCE_DFS_DIR", e1.getMessage());
-

Review Comment:
   I wonder if it is better to try and create some fake properties for the tests or something to check the resolve method vs leaving in stubs for non existent properties. Seems like if we want to check the resolve method we should create a test class specifically for that.



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

To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org

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