keith-turner commented on code in PR #4418:
URL: https://github.com/apache/accumulo/pull/4418#discussion_r1546813163
##########
test/src/main/java/org/apache/accumulo/test/functional/TabletManagementIteratorIT.java:
##########
@@ -165,72 +177,92 @@ public void test() throws AccumuloException,
AccumuloSecurityException, TableExi
// t3 is hosted, setting to never will generate a change to unhost
tablets
setTabletAvailability(client, metaCopy1, t3,
TabletAvailability.UNHOSTED.name());
tabletMgmtParams = createParameters(client);
- assertEquals(4, findTabletsNeedingAttention(client, metaCopy1,
tabletMgmtParams),
+ assertEquals(4, findTabletsNeedingAttention(client, metaCopy1,
tabletMgmtParams).size(),
Review Comment:
Should be able to create the map here instead of using 4. The tablets for
tables t1 and t3 will need location update. Maybe something like the following.
```suggestion
assertEquals(Map.of(endR1, Set.of(NEEDS_LOCATION_UPDATE), prevR1,
Set.of(NEEDS_LOCATION_UPDATE), endR3, Set.of(NEEDS_LOCATION_UPDATE), prevR3,
Set.of(NEEDS_LOCATION_UPDATE)), findTabletsNeedingAttention(client, metaCopy1,
tabletMgmtParams),
"Should have four tablets with hosting availability changes");
```
##########
test/src/main/java/org/apache/accumulo/test/functional/TabletManagementIteratorIT.java:
##########
@@ -141,15 +142,26 @@ public void test() throws AccumuloException,
AccumuloSecurityException, TableExi
// examine a clone of the metadata table, so we can manipulate it
copyTable(client, AccumuloTable.METADATA.tableName(), metaCopy1);
+ // Create expected KeyExtents to test output of
findTabletsNeedingAttention
+ KeyExtent endR1 = new KeyExtent(TableId.of("1"), new Text("some split"),
null);
+ KeyExtent endR3 = new KeyExtent(TableId.of("3"), new Text("some split"),
null);
+ KeyExtent endR4 = new KeyExtent(TableId.of("4"), new Text("some split"),
null);
+ KeyExtent prevR1 = new KeyExtent(TableId.of("1"), null, new Text("some
split"));
+ KeyExtent prevR3 = new KeyExtent(TableId.of("3"), null, new Text("some
split"));
+ KeyExtent prevR4 = new KeyExtent(TableId.of("4"), null, new Text("some
split"));
Review Comment:
Table ids may change, good to resolve them at runtime. Could do something
like the following.
```suggestion
var tableId1 = getServerContext().getTableId(t1);
var tableId3 = getServerContext().getTableId(t3);
var tableId4 = getServerContext().getTableId(t4);
// Create expected KeyExtents to test output of
findTabletsNeedingAttention
KeyExtent endR1 = new KeyExtent(tableId1, new Text("some split"),
null);
KeyExtent endR3 = new KeyExtent(tableId3, new Text("some split"),
null);
KeyExtent endR4 = new KeyExtent(tableId4, new Text("some split"),
null);
KeyExtent prevR1 = new KeyExtent(tableId1, null, new Text("some
split"));
KeyExtent prevR3 = new KeyExtent(tableId3, null, new Text("some
split"));
KeyExtent prevR4 = new KeyExtent(tableId4, null, new Text("some
split"));
```
##########
test/src/main/java/org/apache/accumulo/test/functional/TabletManagementIteratorIT.java:
##########
@@ -165,72 +177,92 @@ public void test() throws AccumuloException,
AccumuloSecurityException, TableExi
// t3 is hosted, setting to never will generate a change to unhost
tablets
setTabletAvailability(client, metaCopy1, t3,
TabletAvailability.UNHOSTED.name());
tabletMgmtParams = createParameters(client);
- assertEquals(4, findTabletsNeedingAttention(client, metaCopy1,
tabletMgmtParams),
+ assertEquals(4, findTabletsNeedingAttention(client, metaCopy1,
tabletMgmtParams).size(),
"Should have four tablets with hosting availability changes");
// test the assigned case (no location)
removeLocation(client, metaCopy1, t3);
- assertEquals(2, findTabletsNeedingAttention(client, metaCopy1,
tabletMgmtParams),
+ expected = Map.of(endR1,
Set.of(TabletManagement.ManagementAction.NEEDS_LOCATION_UPDATE),
Review Comment:
Statically importing
`TabletManagement.ManagementAction.NEEDS_LOCATION_UPDATE` would make the test
code more concise because could then write `Set.of(NEEDS_LOCATION_UPDATE)`
--
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-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
